From 7bb7fc1c49dfc6e81affecf2d15643464d14a189 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 11 Sep 2026 11:53:56 -0400 Subject: [PATCH] fix(artifacts): the run's own output is not an input (#207) `discover_artifacts` walked the project with no idea where the run writes, so `-o` (or `-c`) inside `-i` made run N ingest run N-1's `analysis.json` whole and embed it in its own output. Each run squared the last until the analyzer was SIGKILLed decoding tens of GB of its own output -- which reads as a flaky, load-sensitive test suite and had been misdiagnosed as one more than once. `core.analyze` now hands discovery the paths this run writes: the output and cache directories, plus the output files inside them. Matching is on resolved paths, so a relative, `..`-laden or symlinked target excludes the same tree and a target outside the project excludes nothing. A *directory* exclusion that holds the project itself is refused -- it would empty the inventory -- and the file entries cover that degenerate case, so `-o ` and a `--emit neo4j` `graph.cypher` written to the working directory are stable too. `analysis.json` and `graph.cypher` are named once in `codeanalyzer/options` now, so discovery and the writers cannot drift apart. --- codeanalyzer/__main__.py | 4 +- codeanalyzer/artifacts/discovery.py | 54 +++++++++++++++++++++++++- codeanalyzer/core.py | 21 +++++++++- codeanalyzer/neo4j/emit.py | 4 +- codeanalyzer/options/__init__.py | 4 +- codeanalyzer/options/options.py | 7 ++++ test/test_artifact_discovery.py | 60 +++++++++++++++++++++++++++++ test/test_cli.py | 28 +++++++++++++- 8 files changed, 173 insertions(+), 9 deletions(-) diff --git a/codeanalyzer/__main__.py b/codeanalyzer/__main__.py index f310318..9f322dc 100644 --- a/codeanalyzer/__main__.py +++ b/codeanalyzer/__main__.py @@ -39,7 +39,7 @@ def _pin_hash_seed() -> None: from codeanalyzer.core import Codeanalyzer from codeanalyzer.utils import _set_log_level, logger from codeanalyzer.schema import model_dump, model_dump_json, strip_internal_only -from codeanalyzer.options import AnalysisOptions, EmitTarget +from codeanalyzer.options import ANALYSIS_JSON, AnalysisOptions, EmitTarget def _version_callback(value: bool) -> None: @@ -402,7 +402,7 @@ def main( def _write_output(artifacts, output_dir: Path): """Write analysis.json (the single wire format since #118).""" - output_file = output_dir / "analysis.json" + output_file = output_dir / ANALYSIS_JSON # Use Pydantic's model_dump_json() for compact output # Strip internal-only fields here rather than with a field-level Pydantic # `exclude`: the analysis cache shares the serializer and must keep them. diff --git a/codeanalyzer/artifacts/discovery.py b/codeanalyzer/artifacts/discovery.py index 860be03..1794d1f 100644 --- a/codeanalyzer/artifacts/discovery.py +++ b/codeanalyzer/artifacts/discovery.py @@ -3,10 +3,11 @@ import fnmatch import hashlib from pathlib import Path -from typing import Dict, List, Tuple +from typing import Dict, Iterable, List, Tuple from codeanalyzer.schema.ids import artifact_id from codeanalyzer.schema.py_schema import PyArtifact +from codeanalyzer.utils import logger # (glob pattern against the repo-relative POSIX path, format, roles). # First match wins; patterns are checked in order. @@ -67,6 +68,43 @@ } +def _resolve_exclusions(project_dir: Path, exclude_paths: Iterable[Path]) -> List[Path]: + """Resolve the paths a run writes to, dropping any that would take the whole + project with them (#207). + + A *directory* exclusion at or above the project root would empty the + inventory, which is worse than the bug it guards against, so it is refused. + That case is still covered, because the caller also passes the individual + output *files* -- excluding ``/analysis.json`` costs one file + instead of the entire tree. + """ + root = project_dir.resolve() + kept = [] + for given in exclude_paths: + resolved = given.resolve() + if root.is_relative_to(resolved): + logger.warning( + f"Not excluding {resolved} from artifact discovery: it holds the " + f"project itself. Only this run's own output files under it are " + f"skipped; anything else written there is ingested as an artifact." + ) + continue + kept.append(resolved) + return kept + + +def _is_excluded(path: Path, excluded: List[Path]) -> bool: + """True when ``path`` is, or sits inside, one of ``excluded`` (a file entry + matches only itself). Checked on the path as walked *and* on its resolved + form, so an output directory reached through a symlink is caught from either + side.""" + if not excluded: + return False + if any(path.is_relative_to(directory) for directory in excluded): + return True + return any(path.resolve().is_relative_to(directory) for directory in excluded) + + def _classify(rel_posix: str) -> Tuple[str, List[str]] | None: name = rel_posix.rsplit("/", 1)[-1] for pattern, fmt, roles in RULES: @@ -81,6 +119,7 @@ def discover_artifacts( app_name: str, *, capture_text: bool = True, + exclude_paths: Iterable[Path] = (), ) -> Dict[str, PyArtifact]: """Walk the project and return every file as an artifact, sorted by path. @@ -93,15 +132,28 @@ def discover_artifacts( deliberate exception -- it IS rule-matched (a dependency-manifest), so it is captured like any other manifest despite the `.py` suffix. + ``exclude_paths`` names what this run writes -- the ``--output`` and cache + directories, and the output files inside them (#207). Without them a run + whose output lands inside the project ingests the previous run's whole + ``analysis.json``, and each run embeds the one before it until the process is + killed decoding its own output. Matching is on resolved paths, so a + relative, `..`-laden or symlinked target excludes the same tree, and a target + outside the project excludes nothing. A directory that holds the project + itself is refused (it would empty the inventory); the file entries still + cover that case. + ``source`` is the WHOLE file or nothing -- never a prefix (#172). A decodable file is captured in full; ``capture_text=False`` empties ``source`` everywhere (inventory otherwise identical), and an undecodable file gets ``""`` as ``binary``. ``sha256``/``size_bytes`` always reflect the full file regardless.""" out: Dict[str, PyArtifact] = {} + excluded = _resolve_exclusions(project_dir, exclude_paths) for path in sorted(project_dir.rglob("*")): if not path.is_file(): continue + if _is_excluded(path, excluded): + continue rel = path.relative_to(project_dir) if any(part in _IGNORED_DIRS for part in rel.parts): continue diff --git a/codeanalyzer/core.py b/codeanalyzer/core.py index c9f790c..dcd4d34 100644 --- a/codeanalyzer/core.py +++ b/codeanalyzer/core.py @@ -35,7 +35,7 @@ from codeanalyzer.syntactic_analysis.import_resolver import resolve_imports from codeanalyzer.syntactic_analysis.symbol_table_builder import SymbolTableBuilder from codeanalyzer.utils import ProgressBar -from codeanalyzer.options import AnalysisOptions +from codeanalyzer.options import ANALYSIS_JSON, GRAPH_CYPHER, AnalysisOptions, EmitTarget from codeanalyzer.provenance import analyzer_info, repository_info def _artifact_full_text(project_dir: Path, path: str, art) -> str: @@ -572,6 +572,24 @@ def _home_external_symbols(app, app_id, sig_to_id): ) return externals + def _own_output_paths(self) -> List[Path]: + """Where this run writes: the output and cache directories, plus the + output files themselves (#207). + + Artifact discovery skips these, so a run whose ``-o``/``-c`` lands inside + ``-i`` does not ingest its own previous output (each run embedding the + last until the process is killed decoding it). The file entries carry the + degenerate case where the output directory *is* the project root, which + cannot be skipped wholesale without emptying the inventory. + """ + paths: List[Path] = [self.cache_dir] + if self.options.output is not None: + paths += [self.options.output, self.options.output / ANALYSIS_JSON] + if self.options.emit is EmitTarget.NEO4J: + # No -o means the cypher snapshot lands in the working directory. + paths.append((self.options.output or Path.cwd()) / GRAPH_CYPHER) + return paths + def analyze(self) -> Analysis: """Analyze the project and return the v2 ``Analysis`` envelope. @@ -673,6 +691,7 @@ def analyze(self) -> Analysis: app.artifacts = discover_artifacts( self.project_dir, app_name, capture_text=self.options.artifact_text, + exclude_paths=self._own_output_paths(), ) app.dependencies, app.unresolved_imports = build_dependency_view( app.artifacts, diff --git a/codeanalyzer/neo4j/emit.py b/codeanalyzer/neo4j/emit.py index d1875d1..fb2778d 100644 --- a/codeanalyzer/neo4j/emit.py +++ b/codeanalyzer/neo4j/emit.py @@ -31,7 +31,7 @@ from codeanalyzer.neo4j.schema import build_schema_document from codeanalyzer.neo4j.cypher import render_cypher from codeanalyzer.neo4j.project import project -from codeanalyzer.options import AnalysisOptions +from codeanalyzer.options import GRAPH_CYPHER, AnalysisOptions from codeanalyzer.schema import Analysis from codeanalyzer.schema.assign_ids import assign_ids from codeanalyzer.utils import logger @@ -75,6 +75,6 @@ def emit_neo4j(analysis: Analysis, options: AnalysisOptions) -> None: out_dir = options.output if options.output is not None else Path.cwd() out_dir.mkdir(parents=True, exist_ok=True) - target = out_dir / "graph.cypher" + target = out_dir / GRAPH_CYPHER target.write_text(render_cypher(rows, app_name)) logger.info(f"Neo4j graph written to {target}") diff --git a/codeanalyzer/options/__init__.py b/codeanalyzer/options/__init__.py index 4ad7780..2473720 100644 --- a/codeanalyzer/options/__init__.py +++ b/codeanalyzer/options/__init__.py @@ -1,3 +1,3 @@ -from .options import AnalysisOptions, EmitTarget +from .options import ANALYSIS_JSON, GRAPH_CYPHER, AnalysisOptions, EmitTarget -__all__ = ["AnalysisOptions", "EmitTarget"] +__all__ = ["AnalysisOptions", "EmitTarget", "ANALYSIS_JSON", "GRAPH_CYPHER"] diff --git a/codeanalyzer/options/options.py b/codeanalyzer/options/options.py index 804a477..fe55fc9 100644 --- a/codeanalyzer/options/options.py +++ b/codeanalyzer/options/options.py @@ -4,6 +4,13 @@ from enum import Enum +# The files a run writes into its output directory. Named once so artifact +# discovery can recognize -- and skip -- the run's own output when it lands +# inside the analyzed project (#207). +ANALYSIS_JSON = "analysis.json" +GRAPH_CYPHER = "graph.cypher" + + class EmitTarget(str, Enum): """Output target selected by ``--emit``. diff --git a/test/test_artifact_discovery.py b/test/test_artifact_discovery.py index 077d825..d46d4f5 100644 --- a/test/test_artifact_discovery.py +++ b/test/test_artifact_discovery.py @@ -231,3 +231,63 @@ def test_discovers_terraform_flaskenv_properties_and_generic_ini(tmp_path): assert arts["mypy.ini"].format == "ini" and arts["mypy.ini"].roles == ["tool-config"] # tox.ini keeps matching its own specific (pre-existing) rule, unshadowed. assert arts["tox.ini"].format == "ini" and arts["tox.ini"].roles == ["tool-config"] + + +# --- the run's own output is not an input (#207) -------------------------- + + +def test_output_dir_inside_the_project_is_excluded(tmp_path): + """#207: with `-o` inside `-i`, run N would otherwise ingest run N-1's + `analysis.json` whole -- each run embedding the previous one, unbounded. + The exclusion covers the directory, not a name: any `-o` target works.""" + _mk(tmp_path, "notes.md", "hi\n") + _mk(tmp_path, "out/analysis.json", '{"schema_version": "2.0.0"}\n') + _mk(tmp_path, "out/nested/graph.cypher", "MERGE (n)\n") + arts = discover_artifacts(tmp_path, "a", exclude_paths=[tmp_path / "out"]) + assert sorted(arts) == ["notes.md"] + + +def test_exclusion_is_by_resolved_path_not_by_string(tmp_path): + """The output directory arrives relative, or through a symlink, or with a + `..` in it. All three name the same tree and must all exclude it.""" + _mk(tmp_path, "notes.md", "hi\n") + _mk(tmp_path, "real/analysis.json", "{}\n") + (tmp_path / "link").symlink_to(tmp_path / "real", target_is_directory=True) + for given in ( + tmp_path / "real", + tmp_path / "link", # symlink to the real output dir + tmp_path / "notes.md" / ".." / "real", + ): + arts = discover_artifacts(tmp_path, "a", exclude_paths=[given]) + assert sorted(arts) == ["notes.md"], f"{given} did not exclude the tree" + + +def test_exclusion_outside_the_project_drops_nothing(tmp_path): + """The normal case -- `-o` somewhere else entirely -- must be inert.""" + _mk(tmp_path, "proj/notes.md", "hi\n") + _mk(tmp_path, "proj/Dockerfile", "FROM python:3.12\n") + arts = discover_artifacts( + tmp_path / "proj", "a", exclude_paths=[tmp_path / "elsewhere"] + ) + assert sorted(arts) == ["Dockerfile", "notes.md"] + + +def test_a_file_exclusion_drops_only_that_file(tmp_path): + """`-o` pointing at the project root cannot be skipped as a directory, so the + run's own output file is excluded by name instead (#207).""" + _mk(tmp_path, "notes.md", "hi\n") + _mk(tmp_path, "analysis.json", '{"schema_version": "2.0.0"}\n') + arts = discover_artifacts( + tmp_path, "a", exclude_paths=[tmp_path, tmp_path / "analysis.json"] + ) + assert sorted(arts) == ["notes.md"] + + +def test_exclusion_never_empties_the_inventory(tmp_path): + """`-o` pointing at the project root (or above it) would exclude the whole + project. Dropping every artifact is worse than the bug, so such an + exclusion is refused: the inventory survives intact.""" + _mk(tmp_path, "notes.md", "hi\n") + for given in (tmp_path, tmp_path.parent): + arts = discover_artifacts(tmp_path, "a", exclude_paths=[given]) + assert sorted(arts) == ["notes.md"], f"{given} emptied the inventory" diff --git a/test/test_cli.py b/test/test_cli.py index ed857a2..3f65fb3 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -298,4 +298,30 @@ def _flatten_class(cls: dict) -> list: result.extend(_flatten_callable(method)) for inner in cls.get("types", {}).values(): result.extend(_flatten_class(inner)) - return result \ No newline at end of file + return result + +def test_output_dir_inside_input_does_not_grow_across_runs(cli_runner, tmp_path): + """#207: `-o` inside `-i` made run N ingest run N-1's `analysis.json` whole, + so repeated runs compounded until the analyzer was SIGKILLed decoding its own + output. Two runs, same flags: identical bytes, and the output directory never + appears in the artifact inventory while a sibling file still does.""" + proj = tmp_path / "proj" + proj.mkdir() + (proj / "app.py").write_text("def f(x):\n return x + 1\n") + (proj / "notes.md").write_text("# notes\n") + out = proj / ".output" + + sizes = [] + for i in (1, 2): + result = cli_runner.invoke( + app, + ["-i", str(proj), "-o", str(out), "-a", "1", "--no-venv", + "-c", str(tmp_path / f"cache{i}")], + env={"NO_COLOR": "1", "TERM": "dumb"}, + ) + assert result.exit_code == 0, result.output + sizes.append((out / "analysis.json").stat().st_size) + + assert sizes[0] == sizes[1], f"output grew across runs: {sizes}" + arts = json.loads((out / "analysis.json").read_text())["application"]["artifacts"] + assert sorted(arts) == ["notes.md"]