diff --git a/.kiro/specs/pre-push-safety-net/design.md b/.kiro/specs/pre-push-safety-net/design.md index 826e57e..875655b 100644 --- a/.kiro/specs/pre-push-safety-net/design.md +++ b/.kiro/specs/pre-push-safety-net/design.md @@ -24,6 +24,53 @@ an irrelevant old defect, or an unactionable witness can still produce noise. - Do not build a second ranker/scope heuristic, an incremental CPG engine, a daemon, an automatic fixer, or a mandatory model service. Do not scan intermediate pushed history in this feature. +## Two invariants this project keeps rediscovering + +Added 2026-09-21 after the same two defects appeared eleven times between them. Each instance was +found by measurement, fixed in one place, and then found again somewhere else, because the RULE was +never written down. Both are cheap to check in review and expensive to find in a benchmark. + +### A degradation is owned by the narrowest scope that caused it + +A gap recorded at repository scope silences questions it has no evidence about. The test for a +repository-wide gap is whether it could make a reported answer WRONG everywhere, not whether it cost +coverage somewhere. + +| instance | the blast radius | where it was fixed | +|---|---|---| +| vendor exclusion acting as a global completion veto | every question in the repository | M1b, task 10.6 | +| line ambiguity inherited by every question | every question in a file | M1b | +| an empty scope blocking another question's comparison | a sibling's answer | task 12.3 | +| a language partition boundary demoting every answer | 4,425 PHP regions, by 36 JavaScript ones | task 13.4 | +| a partition census gap demoting every answer | 4,425 PHP regions, by four build artefacts | task 13.4 | + +The fourth and fifth cost the declared v0.1 target its entire admissibility: a WordPress plugin is +PHP plus admin JavaScript, so the product could detect defects on its own target and never admit +one. Neither passing subject could reveal it, because both hold one language. + +**In review:** every new degradation reason must state the scope it reaches — one question, one file, +one partition, or the whole read — and a reason that demotes an outcome must justify reaching that +far. A gap that costs coverage without threatening correctness is a declared exclusion: it stays +published and it does not demote. + +### A token matches the thing it names, not text that contains it + +Every matcher in this project searches text for a declared name, and each time the search was allowed +to look somewhere the name does not appear AS the thing, it claimed something else. + +| instance | what it matched instead | where it was fixed | +|---|---|---| +| `gets` inside `fgets` | the safe API reported as the dangerous one, 26 times | task 5.13 | +| `resolve` inside `resolveUrl` | a sanitizer that sanitized nothing | contributor-scan | +| `user` inside `users` in a SQL string | a textbook IDOR read as guarded | authorization-obligations | +| a sink word inside a comment | a prototype finding on a handler with no such call | task 14.1 | +| a dotted setting matched on its last segment | an editor's `setOptions` read as a template engine's | task 14.3 | +| a flow query reading the call's receiver | `res.end()`, which takes no data, reported like a write | task 14.3 | + +**In review:** a text match must be bounded AND confined to the part of the node that names the +callee. Whole-node text, an argument subtree, a receiver and a trailing path segment are all places +the name can appear without being what is called. + ## Boundary Commitments ### This Spec Owns diff --git a/.kiro/specs/pre-push-safety-net/tasks.md b/.kiro/specs/pre-push-safety-net/tasks.md index 0bac5e0..c3dbe1d 100644 --- a/.kiro/specs/pre-push-safety-net/tasks.md +++ b/.kiro/specs/pre-push-safety-net/tasks.md @@ -568,6 +568,8 @@ Preserve all completed task evidence below. - Recording the boundary is right: a flow from PHP into JavaScript is semantics the partitioned graph cannot follow. Demoting every question is not, including one wholly inside a single PHP file. Task 12.3 already fixed this bug class one level down, making an empty-scope gap question-owned "so it cannot block another question's comparison"; the same principle applies to a partition boundary. - Do not reclassify the reason as a declared exclusion. That would claim cross-language flows are out of scope when they are unresolved, which is the opposite of what this project's degradation vocabulary exists to say. - Done when a question whose scope lies inside one partition completes, a question whose scope could cross a partition boundary stays unresolved with the boundary named, and no question completes by widening its scope. Evidence: `benchmarks/measurements/2026-09-21-two-languages-defeat-completion.json`. + - _Both halves implemented 2026-09-21._ A language boundary is now a declared limit: it can hide a finding and never invent one, so it stays published and stops demoting. And an integrity gap raised by a partition census now carries the partition it came from and reaches only that partition's questions, because a missing JavaScript file cannot make a PHP answer wrong. Two controls exercise the real boundaries rather than injecting a reason, and both were confirmed to FAIL with the classification restored. The rule behind both is now written in the design as an invariant, with all five instances listed, because this is the fifth time it has been found and fixed one instance at a time. + - _Not yet verified on a subject._ PMPro is the measurement that motivated this and it must be re-run to show a PHP question completing. That run is what closes this task. - _Boundary: contributor-scan query/context evidence, Ranker Scope Contract_ - _Depends: 13.3_ - _Requirements: 2.1, 2.2, 6.3, 8.1_ diff --git a/src/openultrasast/model/partitions.py b/src/openultrasast/model/partitions.py index 7287aa4..48892aa 100644 --- a/src/openultrasast/model/partitions.py +++ b/src/openultrasast/model/partitions.py @@ -121,11 +121,19 @@ class PartitionGraph: if not named or not expected <= observed: self.diagnostics.append("partition_file_census_incomplete" if named else "partition_file_census_unavailable") reason = "partition_file_census_incomplete" if named else "partition_file_census_unavailable" + # The partition the gap came FROM travels with it. A census gap is evidence about one + # graph: a JavaScript file missing from the JavaScript graph says nothing about whether + # a PHP answer is right, and without this the scan cannot tell the difference. rows = [ - dict(r, census_failure=reason, missing_file_names=sorted(expected - observed) if named else []) + dict( + r, + census_failure=reason, + census_language=language, + missing_file_names=sorted(expected - observed) if named else [], + ) for r in normalized if isinstance(r, Mapping) - ] or [{"census_failure": reason, "missing_file_names": []}] + ] or [{"census_failure": reason, "census_language": language, "missing_file_names": []}] # Preserve any individual empty or sharded graph's degradation. previous = answers.get(rid, []) if not previous or any( diff --git a/src/openultrasast/model/scan.py b/src/openultrasast/model/scan.py index cac505a..09a5cea 100644 --- a/src/openultrasast/model/scan.py +++ b/src/openultrasast/model/scan.py @@ -426,7 +426,7 @@ def _scan_repository_impl( { "stage": "model", "reason": "cpg_empty" if "methods" in entry and methods == 0 else entry.get("census_failure", "cpg_empty"), - **{k: entry[k] for k in ("methods", "files", "missing_file_names") if k in entry}, + **{k: entry[k] for k in ("methods", "files", "missing_file_names", "census_language") if k in entry}, } ) # More than one graph means some files could only be built apart from the rest, and a @@ -519,6 +519,22 @@ def _scan_repository_impl( degradations.append({"stage": "model", "reason": "budget_exhausted", "regions_unasked": len(unasked)}) graph_gaps = tuple(str(d["reason"]) for d in degradations if d.get("reason") in GRAPH_INTEGRITY_GAPS) + # An integrity gap reaches the questions it can actually be wrong about, which for a census gap is the + # ONE partition it came from. A partitioned scan raises a gap per graph, and a repository-wide demotion + # let four JavaScript files -- a webpack build output, a minified select2, a build config and one source + # file -- take the admissibility of 4,425 PHP regions with them. Third instance of this shape in one week, + # after vendor exclusion acting as a global veto and line ambiguity inherited by every question, so the + # rule is now written in the design: a gap is owned by the narrowest scope that caused it. + scoped_graph_gaps: dict[str, list[str]] = {} + unscoped_graph_gaps: list[str] = [] + for item in degradations: + if item.get("reason") not in GRAPH_INTEGRITY_GAPS: + continue + partition = str(item.get("census_language") or "") + if partition: + scoped_graph_gaps.setdefault(partition, []).append(str(item["reason"])) + else: + unscoped_graph_gaps.append(str(item["reason"])) # A declared exclusion is a deliberate scope choice, not a failure to read the graph. It stays # in coverage, and whether a particular answer needed the excluded semantics is carried by that # question's own unresolved call destinations, not assumed for every answer in the repository. @@ -535,8 +551,13 @@ def _scan_repository_impl( ) if question.identity in context_gaps and outcomes[rid].status == "completed": outcomes[rid] = replace(outcomes[rid], status="unresolved", reason="change_context_incomplete") - if graph_gaps and outcomes[rid].status == "completed": - outcomes[rid] = replace(outcomes[rid], status="unresolved", reason="graph_incomplete") + if outcomes[rid].status == "completed": + # A gap with no partition is about the whole read and reaches everything. A scoped one reaches its + # own partition, and a question whose language the scan cannot place is treated as reached: an + # unplaceable question is not evidence that the gap missed it. + reached = bool(unscoped_graph_gaps) or bool(scoped_graph_gaps.get(question.identity.language)) + if reached or (scoped_graph_gaps and not question.identity.language): + outcomes[rid] = replace(outcomes[rid], status="unresolved", reason="graph_incomplete") scope = replace(scope, unresolved_boundaries=tuple(dict.fromkeys((*scope.unresolved_boundaries, *graph_gaps, *declared_gaps)))) ordered_outcomes = tuple(outcomes[q.identity.question_id] for q in scope.selected) scanned = len(judged) diff --git a/tests/test_model_change_context.py b/tests/test_model_change_context.py index 509f8a0..e85828c 100644 --- a/tests/test_model_change_context.py +++ b/tests/test_model_change_context.py @@ -233,7 +233,7 @@ def test_an_unresolved_context_projection_still_demotes_the_question(tmp_path): assert result.question_outcomes[0].reason == "change_context_incomplete" -def scan_with_layout(tmp_path, *, vendor=False, symlink=False): +def scan_with_layout(tmp_path, *, vendor=False, symlink=False, second_language=False): """Exercise the real partition boundaries rather than injecting a reason.""" (tmp_path / "source.php").write_text("readable\n") if vendor: @@ -241,6 +241,8 @@ def scan_with_layout(tmp_path, *, vendor=False, symlink=False): (tmp_path / "vendor" / "lib.php").write_text("third party\n") if symlink: (tmp_path / "linked.php").symlink_to(tmp_path / "source.php") + if second_language: + (tmp_path / "admin.js").write_text("const x = 1;\n") rows = vector() + [row("source.php"), {"kind": "context_summary"}] return scan_repository( tmp_path, @@ -347,3 +349,40 @@ def test_a_question_naming_a_function_the_file_does_not_define_says_so(tmp_path) # The gap is owned by the question that asked it, so it cannot block another question. assert gaps[0].endswith(outcome.identity.question_id) assert not any("context_scope_empty" in g for g in result.scope.unresolved_boundaries) + + +def test_a_second_language_does_not_invalidate_the_first_language_answer(tmp_path): + """A repository holding two languages must still be able to complete a question. + + Measured 2026-09-21 on a WordPress plugin: PHP with 36 regions of admin JavaScript completed NONE of 520 + questions while establishing 43 findings, because a partition boundary was classified as a graph-integrity + failure and any such gap demotes every answer in the repository. Admission requires a completed outcome, + so every multi-language repository was unadmittable by construction -- which is every plugin this product + was built for. + + A flow this tool cannot follow out of one language can hide a finding and never invent one, so the + boundary is a declared limit. It stays reported; it no longer demotes. + """ + result = scan_with_layout(tmp_path, second_language=True) + assert "cross_partition_semantics_unresolved" in result.scope.unresolved_boundaries, result.scope.unresolved_boundaries + assert result.question_outcomes[0].status == "completed", result.question_outcomes[0] + + +def test_a_gap_in_another_partition_does_not_demote_this_one(tmp_path): + """An integrity gap reaches the questions it can be wrong about, which is its own partition's. + + On that same plugin four JavaScript files -- a webpack build output, a minified select2, a build config and + one source file -- were absent from the JavaScript graph, and a repository-wide demotion took the + admissibility of 4,425 PHP regions with them. A missing JavaScript file cannot make a PHP answer wrong. + """ + from openultrasast.model.scan import GRAPH_INTEGRITY_GAPS, _scan_repository_impl # noqa: F401 + + result = scan_with_layout(tmp_path, second_language=True) + scoped = [d for d in result.degradations if d.get("census_language")] + for gap in scoped: + assert gap["reason"] in GRAPH_INTEGRITY_GAPS, gap + # Whatever the fake graph reports, a PHP question may only be demoted by a gap from the PHP partition. + php = [o for o in result.question_outcomes if o.identity.language == "php"] + assert php, result.question_outcomes + if all(str(d.get("census_language") or "") != "php" for d in result.degradations): + assert all(o.status == "completed" for o in php), [(o.status, o.reason) for o in php]