diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index 826f131..9c9a00b 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -580,10 +580,24 @@ One oversized file empties the whole graph, and `joern-parse` propagates none of the warnings -- thirteen lines of output, exit 0, "Successfully wrote graph". It is also a **race**, not a threshold: five builds of the same two-file input gave three graphs and two empty ones. - - Two things follow. The engine now says so (`cpg_empty`, from a census carried on the batched query; see - `cpg/backend.py`), which is what turned this from a clean bill of health into a diagnosis. And the - mitigation is per-file, not per-repository: `--exclude` the file whose dump is oversized and the rest of - the tree builds, so the coverage disclosure should name the excluded files rather than the build failing. + - **Corrected 2026-09-09, and the first diagnosis above was WRONG.** The banner-interleaving story is not + what happens. Capturing the exact bytes php2cpg receives -- by putting a logging wrapper on `php` -- shows + the parser's output arriving COMPLETE and VALID: two well-formed JSON documents, 5,176,594 bytes, + byte-identical across runs. The defect is inside php2cpg's own reading of that input, and nothing outside + it repairs a given attempt. Tried and each changed nothing: an unbuffered PHP shim, compacting the JSON + to a fifth of its size (5,176,594 -> 943,028), and `ForkJoinPool.common.parallelism=1`. + - What IS true and useful: the failure is **intermittent**, and `joern-parse` is both worse at it and + silent about it. On one WordPress slice, measured: + + joern-parse 5 of 8 builds usable, and says nothing when they are not + php2cpg 11 of 12 builds usable, and names every file it dropped + + - So php now builds through the frontend directly and retries while it reports dropped files + (`_build_with_retries`, 4 attempts). Retrying is only sound because the failure is intermittent AND + reported -- retrying a silent failure would be superstition. Driver-level result on the slice that + previously needed four manual retries: **6 of 6 builds usable**, 3 entailed findings each time. + - Still open: a file that fails on every attempt is still reported rather than analysed, and `--exclude` + plus a second CPG for the excluded files is the remaining lever. - Still open: wiring that exclusion loop, the 637-file build with it in place, and whether the REST handler two calls away can reach the sink -- the slice found it through a same-function flow, not across the object. diff --git a/benchmarks/repos/README.md b/benchmarks/repos/README.md index 2d70c4c..4c9a920 100644 --- a/benchmarks/repos/README.md +++ b/benchmarks/repos/README.md @@ -177,9 +177,24 @@ is still draining from a block-buffered stdout. The banner lands inside the JSON `joern-parse` propagates none of it: thirteen lines of output, exit status 0, "Successfully wrote graph". One oversized file empties the entire graph, so this was never the scale limit it looked like. -It is also a **race**, not a threshold. Five builds of the same two-file input: three produced a graph, two -came back empty. Before the batched query started carrying a census of the graph, two of every five scans of -that plugin would have reported a clean bill of health. +It is also **intermittent**, not a threshold. Five builds of the same two-file input: three produced a graph, +two came back empty. Before the batched query started carrying a census of the graph, two of every five scans +of that plugin would have reported a clean bill of health. + +The first explanation offered here — stderr banners interleaving into buffered stdout — was wrong, and worth +recording as wrong. Capturing the exact bytes php2cpg receives shows the parser's output arriving complete +and valid, byte-identical across runs; the defect is inside php2cpg's own reading of it. An unbuffered shim, +compacting the JSON to a fifth of its size, and pinning ForkJoin parallelism each changed nothing. + +What did work is not a repair but a measurement. `joern-parse` is worse at this than the frontend it calls, +and silent about it: + + joern-parse 5 of 8 builds usable, and says nothing when they are not + php2cpg 11 of 12 builds usable, and names every file it dropped + +So PHP builds through `php2cpg` directly and retries while it reports dropped files. Retrying is sound only +because the failure is intermittent *and* reported — retrying a silent failure would be superstition. The +slice that previously needed four manual retries now builds 6 times out of 6. That is the argument for why a corpus of benchmarks is not enough on its own, made concretely. Every gate stayed byte-identical through all of it. A corpus tells you a verdict is right; it cannot tell you the diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index 4105683..08ef534 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -63,6 +63,23 @@ _FRONTENDS = { "c_cpp": "c2cpg", } +# Languages where the frontend is run DIRECTLY instead of through `joern-parse`, and how many times. +# +# `joern-parse` invokes the same frontend, but measured on one WordPress slice it succeeds far less often +# and, worse, tells you nothing when it does not: thirteen lines of output, exit status 0, "Successfully +# wrote graph", and a graph with no methods in it. The frontend logs a warning per file it drops. +# +# joern-parse 5 of 8 builds usable +# php2cpg 11 of 12 builds usable, and names the files it dropped +# +# php2cpg 4.0.623 fails intermittently when it reads its PHP parser's output: the parser's bytes are +# complete and valid -- verified by capturing them -- so the defect is inside the frontend's own JSON +# reading, and nothing outside it can repair a given attempt. What CAN be done is notice (the warning is +# right there) and try again. Compacting the parser's output to a fifth of its size, running the parser +# through a shim, and pinning `ForkJoinPool.common.parallelism=1` were each tried and each changed nothing. +_PREFER_FRONTEND = frozenset({"php"}) +FRONTEND_BUILD_ATTEMPTS = 4 + Runner = Callable[..., subprocess.CompletedProcess[str]] @@ -173,6 +190,20 @@ class JoernBackend: return None scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-")) cpg_path = scratch / "cpg.bin" + + # A frontend whose failures are visible and retryable is worth more than one whose are not. + if language.lower() in _PREFER_FRONTEND: + retried = self._build_with_retries(root, cpg_path, scratch, language) + if retried is not None: + unparsed = retried + if unparsed: + logger.warning("the frontend could not parse %d file(s) under %s: %s", len(unparsed), root, ", ".join(unparsed[:5])) + return CpgResult( + cpg_path=cpg_path, + run=lambda query, params: self.query(cpg_path, query, params), + run_batch=lambda query, requests: self.query_batch(cpg_path, query, requests), + unparsed=unparsed, + ) command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)] completed = self._run(command, timeout=self.build_timeout, cwd=scratch) unparsed = _unparsed_files(completed) @@ -192,6 +223,33 @@ class JoernBackend: unparsed=unparsed, ) + def _build_with_retries(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None: + """Build through the frontend, trying again while it reports files it could not parse. + + Retrying is only sound because the failure is INTERMITTENT and the frontend says when it happened. + Retrying a silent failure would be superstition; retrying a reported one is just using the report. + The loop stops at the first clean build, and returns the last attempt's dropped files rather than + pretending a partial graph is a whole one. + + ``None`` means there was nothing to build with, so the caller falls back to ``joern-parse``. + """ + best: tuple[str, ...] | None = None + for attempt in range(FRONTEND_BUILD_ATTEMPTS): + unparsed = self._build_with_frontend(root, cpg_path, scratch, language) + if unparsed is None: + return best # the frontend is missing or failed outright; keep any earlier partial build + if not unparsed: + return () + best = unparsed + logger.info( + "frontend dropped %d file(s) on attempt %d of %d for %s; retrying", + len(unparsed), + attempt + 1, + FRONTEND_BUILD_ATTEMPTS, + root, + ) + return best + def _build_with_frontend(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None: """Retry the build through the language frontend alone. diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index bdf3af2..9b96c4d 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -265,7 +265,11 @@ def test_the_operator_can_raise_the_heap(tmp_path: Path, monkeypatch: pytest.Mon def test_a_failed_joern_parse_retries_through_the_language_frontend(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Measured on a 637-file WordPress plugin: `joern-parse` threw after applying overlays while `php2cpg` on the same tree produced a 4.3MB CPG with no errors. Joern's own message recommends the direct route for - a large codebase, so this is the documented fallback rather than a workaround.""" + a large codebase, so this is the documented fallback rather than a workaround. + + Uses a language OUTSIDE `_PREFER_FRONTEND`, because php no longer waits for joern-parse to fail first -- + see the test below. + """ from openultrasast.cpg.backend import JoernBackend tried: list[str] = [] @@ -286,10 +290,10 @@ def test_a_failed_joern_parse_retries_through_the_language_frontend(tmp_path: Pa monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on") monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}") - result = JoernBackend(runner=runner).build(tmp_path, language="php") + result = JoernBackend(runner=runner).build(tmp_path, language="python") assert result is not None, "the frontend succeeded where joern-parse did not" - assert tried == ["joern-parse", "php2cpg"] + assert tried == ["joern-parse", "pysrc2cpg"] def test_an_unknown_language_has_nothing_to_retry_with(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -380,3 +384,58 @@ def test_a_joern_log_line_inside_the_fence_does_not_destroy_the_payload() -> Non noisy = f'{BEGIN}\n[INFO ] Attempting to determine flows from empty list of sources.\n{{"0": [], "1": [1]}}\n{END}\n' assert extract_payload(noisy) == {"0": [], "1": [1]} + + +def test_php_builds_through_the_frontend_and_retries_while_files_are_dropped(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """php2cpg fails INTERMITTENTLY, and unlike joern-parse it says so. + + Measured on one WordPress slice: joern-parse produced a usable graph 5 times in 8 and reported nothing + when it did not -- exit 0, "Successfully wrote graph", no methods in the graph. php2cpg on the same tree + managed 11 in 12 and named every file it dropped. So php goes straight to the frontend, and a build that + reports dropped files is simply attempted again. + """ + import subprocess + + from openultrasast.cpg.backend import JoernBackend + + tried: list[str] = [] + warning = "WARN AstCreationPass Failed to process '/src/big.php'\n" + + def runner(command, **kwargs): # type: ignore[no-untyped-def] + tried.append(Path(command[0]).name) + Path(command[command.index("-o") + 1]).write_text("cpg") + # Drops a file on the first two attempts, clean on the third. + stderr = warning if len(tried) <= 2 else "" + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr=stderr) + + monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on") + monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}") + + result = JoernBackend(runner=runner).build(tmp_path, language="php") + + assert result is not None + assert tried == ["php2cpg", "php2cpg", "php2cpg"], "joern-parse is not consulted for php, and it retried" + assert result.unparsed == (), "the third attempt was clean, so nothing is reported as dropped" + + +def test_a_php_build_that_never_stops_dropping_files_reports_them(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Retrying is not the same as pretending. When every attempt drops the same file, the caller is told.""" + import subprocess + + from openultrasast.cpg.backend import FRONTEND_BUILD_ATTEMPTS, JoernBackend + + attempts = 0 + + def runner(command, **kwargs): # type: ignore[no-untyped-def] + nonlocal attempts + attempts += 1 + Path(command[command.index("-o") + 1]).write_text("cpg") + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="WARN Failed to process '/src/stubborn.php'\n") + + monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on") + monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}") + + result = JoernBackend(runner=runner).build(tmp_path, language="php") + + assert attempts == FRONTEND_BUILD_ATTEMPTS, "it gives up rather than retrying forever" + assert result is not None and result.unparsed == ("/src/stubborn.php",)