From aca1eecf05e656a2cc87ef03923b852c1e9b9d02 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 04:21:17 +0530 Subject: [PATCH 1/8] fix(harness): give a browser scan its limits cut a second look The browser scan stopped reading an agent's code at any of its limits (file size, the scan, script and Python totals, the parse total, the token total and work, the decode and search totals, a skipped file, a package.json it couldn't read) and returned what it had, so padding read first or a large honest repository hid a plain browser import, silently. The scan now returns the limits that cut it, and a cut reading gets one second look with its own message before the model's answer stands; a cut never refuses on its own. One WARNING per cut scan names the limits and counts. The import walk's own stops are left to its own fix. Refs: TH-9512 Signed-off-by: khushalsonawat --- .github/workflows/sdk-smoke.yml | 1 + src/fi/alk/harness/HOW-IT-WORKS.md | 3 +- src/fi/alk/harness/agent_type_check.py | 174 +++++- src/fi/alk/harness/tools.py | 29 +- tests/harness/test_agent_type_refusal.py | 61 +- tests/harness/test_browser_scan_cut.py | 588 ++++++++++++++++++ .../harness/test_contract_tools_subfolder.py | 14 +- 7 files changed, 792 insertions(+), 78 deletions(-) create mode 100644 tests/harness/test_browser_scan_cut.py diff --git a/.github/workflows/sdk-smoke.yml b/.github/workflows/sdk-smoke.yml index ec3c66a73..7c5159091 100644 --- a/.github/workflows/sdk-smoke.yml +++ b/.github/workflows/sdk-smoke.yml @@ -100,6 +100,7 @@ jobs: tests/harness/test_reading_entry.py tests/harness/test_agent_reading.py tests/harness/test_scan_walk.py + tests/harness/test_browser_scan_cut.py tests/harness/test_scan_folders.py tests/harness/test_scan_document.py tests/harness/test_scan_ai.py diff --git a/src/fi/alk/harness/HOW-IT-WORKS.md b/src/fi/alk/harness/HOW-IT-WORKS.md index 6e40b22e8..f18c19b26 100644 --- a/src/fi/alk/harness/HOW-IT-WORKS.md +++ b/src/fi/alk/harness/HOW-IT-WORKS.md @@ -80,7 +80,8 @@ conversation**, so the model corrects and resubmits rather than a bad contract l A browser reading gets one second look first, naming any browser, crawling or computer-use tool the code check found in the source. So does a chat or voice reading of source -where the check found one. The next submission stands: a browser answer is refused to +where the check found one, or where the check stopped before reading all of it. The next +submission stands: a browser answer is refused to `failure.json` and the build exits 78, and a chat or voice answer is saved. The look gives a value to send back, which only tells apart calls in one model message: those run before the model reads any result. The rule and its state are `SecondLook` in `agent_type_check.py`. A browser answer to a chat or voice look is refused at once, since that look said browser stops the build. When calls in one message run one after another, or together once the first scan is done, a later call is taken as the answer before the model has read the look. diff --git a/src/fi/alk/harness/agent_type_check.py b/src/fi/alk/harness/agent_type_check.py index 1679bb511..1a6ff5141 100644 --- a/src/fi/alk/harness/agent_type_check.py +++ b/src/fi/alk/harness/agent_type_check.py @@ -15,6 +15,7 @@ import os import re import tokenize +from collections import Counter from collections.abc import Iterable, Iterator, Sequence from functools import lru_cache from pathlib import Path @@ -361,14 +362,16 @@ def _agent_files( yield Path(path) -def _packages_used(path: Path) -> set[str]: +def _packages_used(path: Path) -> set[str] | None: + """The browser packages ``path`` lists, or None when it can't be read: npm may still read it.""" try: + # A link to nothing or a pipe isn't a manifest npm reads either. if not path.is_file(): return set() # Windows editors often save a BOM, which json refuses. manifest = json.loads(path.read_text(encoding="utf-8").removeprefix("\ufeff")) except (OSError, ValueError, RecursionError): - return set() + return None if not isinstance(manifest, dict): return set() names: set[str] = set() @@ -384,13 +387,14 @@ def _packages_used(path: Path) -> set[str]: return found -def _imports_in(text: str, path: Path) -> set[str]: +def _imports_in(text: str, path: Path) -> set[str] | None: + """The modules ``text`` imports, or None when it won't parse: then they're unknown, not none.""" # One file at a time: a customer's odd file can run the parser out of memory, and that mustn't # take the rest of the scan, or the reading it serves, down with it. tree = parsed(text) if tree is None: logger.debug("agent type scan read no imports from %s", path) - return set() + return None return _imports(tree) @@ -432,14 +436,15 @@ def _script_kept(hit: re.Match[str]) -> str: return kept -def _python_markers(text: str, tokens: TokenBudget, path: Path) -> set[str]: +def _python_markers(text: str, tokens: TokenBudget, path: Path) -> set[str] | None: + """The tool markers in ``text``'s code, or None when the scan's tokenizing ran out first.""" if not _MARKER_START.search(text): return set() if not tokens.spend(text): # Past the total, comments can't be told from code, and a marker in one would refuse a # browser reading without its second look; so the file's markers aren't read, and the - # model's reading stands. - return set() + # scan says it was cut. + return None try: found = tokens.tokens(text) except (ValueError, SyntaxError, tokenize.TokenError) as error: @@ -447,7 +452,7 @@ def _python_markers(text: str, tokens: TokenBudget, path: Path) -> set[str]: logger.debug("agent type scan read no markers from %s: %s", path, type(error).__name__) return set() # Past the work, too, the file's markers aren't read. - return set() if found is None else _markers(_python_code(found)) + return None if found is None else _markers(_python_code(found)) def _markers(code: str) -> set[str]: @@ -486,7 +491,26 @@ def browser_tools_found(source_root: Path | None) -> list[str]: if source_root is None: return [] root = Path(source_root).resolve() - return list(_scan(root, _signature(root))) + return list(_scan(root, _signature(root))[0]) + + +def browser_scan( + source_root: Path | None, subfolder: str | None = None, reached: list[Path] | None = None +) -> tuple[list[str], frozenset[str]]: + """What ``browser_tools_found``, or with ``subfolder`` ``browser_tools_in_agent``, finds, and + the limits that cut the scan (see ``_tools_in``), from one walk: a second walk would cost the + conversation the scan's time again. + + Raises ``SubfolderNotFound`` for a folder that can't be used. + """ + if source_root is None: + return [], frozenset() + if not subfolder: + root = Path(source_root).resolve() + names, cut = _scan(root, _signature(root)) + else: + names, cut = _scanned_agent(source_root, subfolder, reached) + return list(names), cut def _is_read(path: Path) -> bool: @@ -525,7 +549,9 @@ def _signature(root: Path) -> tuple[int, int, int]: @lru_cache(maxsize=8) -def _scan(root: Path, _files: tuple[int, int, int]) -> tuple[str, ...]: +def _scan( + root: Path, _files: tuple[int, int, int] +) -> tuple[tuple[str, ...], frozenset[str]]: """One scan per state of the files: every builder reopen in a process asks again.""" # Each record read once: the walk keeps the top's rows for the modules there. left = [_LARGEST_RECORDS] @@ -539,40 +565,56 @@ def _scan(root: Path, _files: tuple[int, int, int]) -> tuple[str, ...]: def _tools_in( paths: Iterable[Path], local: set[str], first: Path | None = None -) -> tuple[str, ...]: +) -> tuple[tuple[str, ...], frozenset[str]]: """The browser tools ``paths`` import or name, the files in ``first`` read before the rest: - the totals below cut the files read last, and the agent's own mustn't be those.""" + the totals below cut the files read last, and the agent's own mustn't be those. + + And the limits that cut the scan, by name: each left code unread or unparsed, so finding + nothing then doesn't say there's nothing, and the reading gets a second look. A file that + names no tool, or that Python can't decode, isn't one: it can't import what Python runs. + """ found: set[str] = set() imported: set[str] = set() + # A count, not a flag: the log says how much each limit left unread. + cut: Counter[str] = Counter() scanned = scripts = python = 0 budget = ParseBudget() tokens = TokenBudget() escapes = EscapeBudget() searched = ParseBudget(_LARGEST_RAW_SEARCH) - for path in sorted( + ordered = sorted( filter(_is_read, paths), key=lambda path: (first is not None and not is_within(path, first), *_reading_order(path)), - ): + ) + for at, path in enumerate(ordered): size = _file_size(path) # Never read, so it mustn't use up the limit before the code that is. if size > _largest_read(path): + cut["file_size"] += 1 continue scanned += size if scanned > _LARGEST_SCAN: + cut["scan_total"] += len(ordered) - at break if path.suffix in _SCRIPT_SUFFIXES: scripts += size if scripts > _LARGEST_SCRIPT_SCAN: + cut["script_total"] += 1 continue elif path.suffix == ".py": python += size if python > _LARGEST_PYTHON_SCAN: + cut["python_total"] += 1 continue # One file at a time: the tokenizer raises more than it documents (SystemError on a NUL # byte in 3.12), and one odd file mustn't stop every reading from being checked. try: if path.name == "package.json": - found |= _packages_used(path) + packages = _packages_used(path) + if packages is None: + cut["package_unreadable"] += 1 + else: + found |= packages elif path.suffix != ".py": found |= _markers_in(path) elif ( @@ -581,9 +623,16 @@ def _tools_in( ) ) is not None: markers = _python_markers(text, tokens, path) - names = _imports_in(text, path) if budget.spend(text) else set() + if markers is None: + cut["token_total"] += 1 + markers = set() found |= markers - imported |= names + if not budget.spend(text): + cut["parse_total"] += 1 + elif (names := _imports_in(text, path)) is None: + cut["parse_failed"] += 1 + else: + imported |= names elif undecoded: # A codec refused by name, which Python still runs the file in: the browser # packages and tool markers it names, as bytes, count. That leads to a second @@ -599,11 +648,24 @@ def _tools_in( # Whole names only: "steelers" in a string is no import of steel. named = {name for name in named if _WHOLE_NAMES[name].search(raw)} found |= _markers(raw) + else: + # A tool marker it names then goes unseen. + cut["search_total"] += 1 imported |= named except Exception: # A skipped file may be the agent's main one, and the reading then finds nothing. logger.warning("agent type scan skipped %s", path, exc_info=True) + cut["skipped"] += 1 continue + if escapes.left < 0: + # A file past the total was searched as bytes, which can miss a name its codec spells. + cut["decode_total"] = 1 + if cut: + # Names and counts only: a path can name the customer's business. + logger.warning( + "agent type scan cut by %s", + ", ".join(f"{name} {count}" for name, count in sorted(cut.items())), + ) for name in imported: top = name.split(".")[0] if top in local: @@ -612,7 +674,7 @@ def _tools_in( found.add(top) elif name.endswith(f".{_TOOLKIT}"): found.add(_TOOLKIT) - return tuple(sorted({name.replace("_", "-") for name in found})) + return tuple(sorted({name.replace("_", "-") for name in found})), frozenset(cut) def _files_in(folder: Path) -> list[Path]: @@ -682,6 +744,14 @@ def browser_tools_in_agent( Raises ``SubfolderNotFound`` for a folder that can't be used. """ + return list(_scanned_agent(source_root, subfolder, reached)[0]) + + +def _scanned_agent( + source_root: Path, subfolder: str, reached: list[Path] | None +) -> tuple[tuple[str, ...], frozenset[str]]: + """``browser_tools_in_agent``'s scan, and the limits that cut it. The walk's own stops aren't + among them: they're logged by ``_walked``.""" root = Path(source_root).resolve() folder = agent_folder(root, subfolder) above = [ @@ -750,12 +820,8 @@ def unpacked(path: Path) -> bool: for parent in path.parents if is_within(parent, root) and (parent / "package.json").is_file() ] - return list( - _tools_in( - dict.fromkeys((*_agent_files(folder), *above, *shared, *manifests)), - local, - folder, - ) + return _tools_in( + dict.fromkeys((*_agent_files(folder), *above, *shared, *manifests)), local, folder ) @@ -772,18 +838,30 @@ def is_browser(text: str) -> bool: def decide( - claimed: Any, found: Sequence[str], *, looked_again: bool + claimed: Any, + found: Sequence[str], + *, + looked_again: bool, + cut: frozenset[str] = frozenset(), ) -> Literal["refuse", "second_look", "keep"]: """What the model read against what the code check found. A browser reading always gets one - more look first, and so does a reading the check disagrees with; the answer then stands.""" + more look first, and so does a reading the check disagrees with, or one it couldn't check all + of (``cut``, the limits that stopped it); the answer then stands.""" if is_browser(modality_text(claimed)): # Never at once: what was found can be a dev dependency or a helper script of a chat # agent, and a refusal can't be retried. return "refuse" if looked_again else "second_look" - return "second_look" if found and not looked_again else "keep" + # A cut is never a refusal: a limit reached on an honest repository says nothing of its job. + return "second_look" if (found or cut) and not looked_again else "keep" SECOND_LOOK_READ = "second_look_read" +# Shared by both cut texts, which must say the same of the check. +_CHECK_STOPPED = ( + "the code check stopped before reading all of the agent's code (it holds more than the check " + "reads, or a file it couldn't read), so it can't say whether a browser, crawling or " + "computer-use tool is used" +) class SecondLook: @@ -809,7 +887,13 @@ def given(self) -> bool: return bool(self._receipts) def verdict( - self, claimed: str, receipt: Any, found: Sequence[str], *, given_before: bool + self, + claimed: str, + receipt: Any, + found: Sequence[str], + *, + given_before: bool, + cut: frozenset[str] = frozenset(), ) -> Literal["refuse", "second_look", "keep"]: # A call holding a value read a look, so its answer stands. Without one, the call after a # look that no value came back for is taken as the answer. Once a value came back, a call @@ -824,19 +908,28 @@ def verdict( claimed, found, looked_again=read_it or (given_before and (self._unanswered or same_answer)), + cut=cut, ) - def issue(self, claimed: str, found: Sequence[str], receipt: str) -> str: + def issue( + self, + claimed: str, + found: Sequence[str], + receipt: str, + *, + cut: frozenset[str] = frozenset(), + ) -> str: """The look's text, with ``receipt`` as the value to send back.""" self._receipts.add(receipt) self._unanswered = True return ( - f"{self.prompt(claimed, found)} When you submit again, set " + f"{self.prompt(claimed, found, cut=cut)} When you submit again, set " f'{SECOND_LOOK_READ} to "{receipt}", which shows you read this.' ) @staticmethod - def prompt(claimed: str, found: Sequence[str]) -> str: + def prompt(claimed: str, found: Sequence[str], *, cut: frozenset[str] = frozenset()) -> str: + # What was found already says what to decide, cut or not, so its text is the same. if is_browser(claimed) and found: return ( "second-look: you recorded modality browser, which stops this build as not " @@ -846,6 +939,16 @@ def prompt(claimed: str, found: Sequence[str]) -> str: "For a conversation, submit again as chat or voice. If its main job is that work, " "submit again as browser. Decide this from the source; don't ask anyone." ) + if is_browser(claimed) and cut: + # Nothing found in code left unread isn't nothing found. + return ( + "second-look: you recorded modality browser, which stops this build as not " + f"supported, and {_CHECK_STOPPED}. A chat or voice agent can use one in its tests " + "or a helper script. Does a person come to this agent for a conversation, or to " + "have it carry out work on websites or a computer? For a conversation, submit " + "again as chat or voice. If you're sure, submit again as browser. Decide this " + "from the source; don't ask anyone." + ) if is_browser(claimed): return ( "second-look: you recorded modality browser, which stops this build as not " @@ -855,6 +958,15 @@ def prompt(claimed: str, found: Sequence[str]) -> str: "voice. If you're sure, submit again as browser. Decide this from the source; " "don't ask anyone." ) + if cut and not found: + return ( + f"second-look: you recorded modality {claimed or 'chat'}, and {_CHECK_STOPPED}. " + "A chat or voice agent can use one in its tests or a helper script. If a person " + "comes to this agent for a conversation, submit again as is. If its main job is " + "carrying out work on websites or a computer, submit again as browser, which " + "stops this build as not supported. Decide this from the source; don't ask " + "anyone." + ) return ( f"second-look: you recorded modality {claimed or 'chat'}, and the source uses " f"{', '.join(found)}. If that's a helper and a person comes to this agent for a " diff --git a/src/fi/alk/harness/tools.py b/src/fi/alk/harness/tools.py index 57365734e..ca04bb5b1 100644 --- a/src/fi/alk/harness/tools.py +++ b/src/fi/alk/harness/tools.py @@ -25,8 +25,7 @@ SECOND_LOOK_READ, SecondLook, agent_type_failure, - browser_tools_found, - browser_tools_in_agent, + browser_scan, code_the_agent_reaches, failure_message, is_browser, @@ -465,16 +464,16 @@ def _builds_a_sibling(source_root: Path, runtime: Runtime, subfolder: str) -> st def _tools_in_agent_folder( scan_root: Path | None, subfolder: str | None, reached: list[Path] | None = None -) -> list[str]: - """The browser tools this agent uses, which a sibling's aren't. ``reached`` is filled with - the code it reaches outside its folder.""" +) -> tuple[list[str], frozenset[str]]: + """The browser tools this agent uses, which a sibling's aren't, and the limits that cut the + scan. ``reached`` is filled with the code it reaches outside its folder.""" if not scan_root or not subfolder: - return browser_tools_found(scan_root) + return browser_scan(scan_root) try: - return browser_tools_in_agent(scan_root, subfolder, reached) + return browser_scan(scan_root, subfolder, reached) except SubfolderNotFound: # The folder check reports this; the model's reading shouldn't be refused for it too. - return [] + return [], frozenset() def contract_tools( @@ -517,11 +516,13 @@ def contract_tools( refused = False second_look = SecondLook() found: list[str] | None = None + # Set with found, from the same scan: a later call skips the scan and must still see its cut. + cut: frozenset[str] = frozenset() # Calls in one message check side by side, so the one sent last can finish first; it's the # model's last word, and an earlier call that finishes later mustn't write over it. sent = saved = 0 # Calls in one message share one scan: each running its own takes that many times longer. - scan: asyncio.Future[list[str]] | None = None + scan: asyncio.Future[tuple[list[str], frozenset[str]]] | None = None # The code outside the folder the scan found the agent reaches, for the instrumentation # check, so the save walks the imports once. reach: list[Path] = [] @@ -1311,7 +1312,7 @@ async def submit_contract(args: dict[str, Any]) -> dict[str, Any]: receipt = payload.pop(SECOND_LOOK_READ, None) or ( args.get(SECOND_LOOK_READ) if isinstance(args, dict) else None ) - nonlocal refused, found, sent, saved, scan + nonlocal refused, found, cut, sent, saved, scan # Taken before the scan's wait: a call already running when a look went out hasn't read it. looked_before = second_look.given sent += 1 @@ -1331,7 +1332,7 @@ async def submit_contract(args: dict[str, Any]) -> dict[str, Any]: asyncio.to_thread(_tools_in_agent_folder, scan_root, subfolder, reach) ) # Shielded: one call cancelled mustn't cancel the scan the others wait on. - found = await asyncio.shield(scan) + found, cut = await asyncio.shield(scan) # A call running alongside this one may have refused while this one waited. if refused or (destination / FAILURE_FILE).is_file(): return _ok(_stop_message(destination)) @@ -1343,7 +1344,9 @@ async def submit_contract(args: dict[str, Any]) -> dict[str, Any]: "job is work on websites or a computer." ] ) - verdict = second_look.verdict(claimed, receipt, found, given_before=looked_before) + verdict = second_look.verdict( + claimed, receipt, found, given_before=looked_before, cut=cut + ) if verdict == "refuse": # Set first: if the write fails, the model must not get a resubmission through. refused = True @@ -1355,7 +1358,7 @@ async def submit_contract(args: dict[str, Any]) -> dict[str, Any]: write_failure_file(agent_type_failure(found), destination) return _ok(_stop_message(destination)) if verdict == "second_look": - return _problems([second_look.issue(claimed, found, _new_receipt())]) + return _problems([second_look.issue(claimed, found, _new_receipt(), cut=cut)]) data_store = payload.get("data_store") if not isinstance(data_store, dict): data_store = {} diff --git a/tests/harness/test_agent_type_refusal.py b/tests/harness/test_agent_type_refusal.py index dcb24c5a8..4b5c7cc05 100644 --- a/tests/harness/test_agent_type_refusal.py +++ b/tests/harness/test_agent_type_refusal.py @@ -602,20 +602,21 @@ def odd(path, *args, **kwargs): monkeypatch.setattr(agent_type_check, "read_module", odd) with caplog.at_level("WARNING", logger=agent_type_check.__name__): assert browser_tools_found(tmp_path) == ["playwright"] - [record] = caplog.records - assert "a_odd.py" in record.getMessage() - assert record.exc_info[0] is LookupError + skipped, cut = caplog.records + assert "a_odd.py" in skipped.getMessage() + assert skipped.exc_info[0] is LookupError + assert cut.getMessage() == "agent type scan cut by skipped 1" @pytest.mark.skipif(sys.version_info < (3, 12), reason="3.11's tokenizer reads a NUL byte") def test_a_file_with_a_nul_byte_is_left_unread_without_stopping_the_scan(tmp_path, caplog): # Through the shared reader a NUL byte is a file Python can't parse, an expected answer, not - # an error: it's left unread and nothing is logged. A file naming nothing isn't even read. + # an error: it's left unread and only the cut is logged. A file naming nothing isn't even read. (tmp_path / "a_blob.py").write_bytes(b" x = 1\n\x00") (tmp_path / "b_blob.py").write_bytes(b"import playwright\n\x00") with caplog.at_level("WARNING", logger=agent_type_check.__name__): assert browser_tools_found(tmp_path) == [] - assert caplog.records == [] + assert [r.getMessage() for r in caplog.records] == ["agent type scan cut by parse_failed 1"] def test_a_python_file_with_a_coding_line_keeps_its_marker(tmp_path): @@ -1615,11 +1616,11 @@ def test_unknown_modality_gets_a_problem_not_a_save(tmp_path, plain_repo): def test_the_scan_runs_off_the_event_loop(tmp_path, monkeypatch): threads = [] - def scan(_root): + def scan(_root, subfolder=None, reached=None): threads.append(threading.current_thread()) - return [] + return [], frozenset() - monkeypatch.setattr(tools, "browser_tools_found", scan) + monkeypatch.setattr(tools, "browser_scan", scan) _submitter(tmp_path / "out", tmp_path)(_reading("chat")) assert threads and threads[0] is not threading.main_thread() @@ -1628,11 +1629,11 @@ def scan(_root): def test_a_refusal_during_the_scan_stops_this_call_too(tmp_path, monkeypatch, modality): output = tmp_path / "out" - def scan(_root): + def scan(_root, subfolder=None, reached=None): write_failure_file(agent_type_failure(["playwright"]), output) - return [] + return [], frozenset() - monkeypatch.setattr(tools, "browser_tools_found", scan) + monkeypatch.setattr(tools, "browser_scan", scan) answer = _submitter(output, tmp_path)(_reading(modality)) assert not (output / "contract.json").exists() # The stop itself, not a second look: the reading was refused while the scan ran. A browser @@ -2410,13 +2411,13 @@ def test_conversation_with_no_tool_saves_with_no_second_look(tmp_path, plain_rep def _counted_scans(monkeypatch) -> list[Path]: scans: list[Path] = [] - real_scan = tools.browser_tools_found + real_scan = tools.browser_scan - def counted(root): + def counted(root, subfolder=None, reached=None): scans.append(root) - return real_scan(root) + return real_scan(root, subfolder, reached) - monkeypatch.setattr(tools, "browser_tools_found", counted) + monkeypatch.setattr(tools, "browser_scan", counted) return scans @@ -2463,12 +2464,12 @@ def test_a_call_cancelled_while_waiting_on_the_scan_leaves_it_to_the_others( ): started, release = threading.Event(), threading.Event() - def slow(_root): + def slow(_root, subfolder=None, reached=None): started.set() release.wait(10) - return [] + return [], frozenset() - monkeypatch.setattr(tools, "browser_tools_found", slow) + monkeypatch.setattr(tools, "browser_scan", slow) tool = _submit_tool(contract_tools(tmp_path / "out", scan_root=plain_repo)) async def first_message(): @@ -2484,15 +2485,15 @@ async def first_message(): def test_a_scan_that_fails_is_run_again_by_the_next_call(tmp_path, plain_repo, monkeypatch): - outcomes = iter([OSError("gone"), []]) + outcomes = iter([OSError("gone"), ([], frozenset())]) - def scan(_root): + def scan(_root, subfolder=None, reached=None): outcome = next(outcomes) if isinstance(outcome, Exception): raise outcome return outcome - monkeypatch.setattr(tools, "browser_tools_found", scan) + monkeypatch.setattr(tools, "browser_scan", scan) submit = _submitter(tmp_path / "out", plain_repo) with pytest.raises(OSError, match="gone"): submit(_reading("chat")) @@ -2502,15 +2503,15 @@ def scan(_root): def test_a_scan_that_fails_is_run_again_by_the_next_call_in_the_same_message( tmp_path, plain_repo, monkeypatch ): - outcomes = iter([OSError("gone"), []]) + outcomes = iter([OSError("gone"), ([], frozenset())]) - def scan(_root): + def scan(_root, subfolder=None, reached=None): outcome = next(outcomes) if isinstance(outcome, Exception): raise outcome return outcome - monkeypatch.setattr(tools, "browser_tools_found", scan) + monkeypatch.setattr(tools, "browser_scan", scan) tool = _submit_tool(contract_tools(tmp_path / "out", scan_root=plain_repo)) async def one_message(): @@ -2527,13 +2528,13 @@ def test_a_scan_that_finished_after_its_only_caller_was_cancelled_is_not_run_aga started, release = threading.Event(), threading.Event() scans: list[Path] = [] - def slow(root): + def slow(root, subfolder=None, reached=None): scans.append(root) started.set() release.wait(10) - return [] + return [], frozenset() - monkeypatch.setattr(tools, "browser_tools_found", slow) + monkeypatch.setattr(tools, "browser_scan", slow) tool = _submit_tool(contract_tools(tmp_path / "out", scan_root=plain_repo)) async def cancelled_then_next(): @@ -2553,14 +2554,14 @@ def test_a_scan_left_running_on_a_closed_loop_is_started_again(tmp_path, plain_r started, release = threading.Event(), threading.Event() scans: list[Path] = [] - def scan(root): + def scan(root, subfolder=None, reached=None): scans.append(root) if len(scans) == 1: started.set() release.wait(10) - return [] + return [], frozenset() - monkeypatch.setattr(tools, "browser_tools_found", scan) + monkeypatch.setattr(tools, "browser_scan", scan) tool = _submit_tool(contract_tools(tmp_path / "out", scan_root=plain_repo)) closed = asyncio.new_event_loop() try: diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py new file mode 100644 index 000000000..4b8961545 --- /dev/null +++ b/tests/harness/test_browser_scan_cut.py @@ -0,0 +1,588 @@ +"""A browser scan that leaves code unread or unparsed says so, and the reading gets a second look. + +Finding nothing in code the scan didn't read isn't finding nothing: padding read first, or an +honest repository larger than the limits, would otherwise hide a plain ``import playwright``. +""" + +from __future__ import annotations + +import asyncio +import json +import logging +from pathlib import Path + +import pytest + +from fi.alk.harness import agent_type_check, python_source, tools +from fi.alk.harness.agent_type_check import ( + SecondLook, + browser_scan, + browser_tools_found, + browser_tools_in_agent, + decide, +) +from fi.alk.harness.failure_file import FAILURE_FILE +from fi.alk.harness.python_source import LARGEST_PARSED_MODULE +from fi.alk.harness.tools import contract_tools +from fi.alk.harness.understand import load + +_READ = "read-it" +_NOT_ACCEPTED = "Not accepted. Fix all of these and call submit_contract again:\n - " +_RECEIPT = f' When you submit again, set second_look_read to "{_READ}", which shows you read this.' +_CUT_CHAT = ( + "second-look: you recorded modality {claimed}, and the code check stopped before reading all " + "of the agent's code (it holds more than the check reads, or a file it couldn't read), so it " + "can't say whether a browser, crawling or computer-use tool is used. A chat or voice agent " + "can use one in its tests or a helper script. If a person comes to this agent for a " + "conversation, submit again as is. If its main job is carrying out work on websites or a " + "computer, submit again as browser, which stops this build as not supported. Decide this " + "from the source; don't ask anyone." +) +_CUT_BROWSER = ( + "second-look: you recorded modality browser, which stops this build as not supported, and " + "the code check stopped before reading all of the agent's code (it holds more than the check " + "reads, or a file it couldn't read), so it can't say whether a browser, crawling or " + "computer-use tool is used. A chat or voice agent can use one in its tests or a helper " + "script. Does a person come to this agent for a conversation, or to have it carry out work on " + "websites or a computer? For a conversation, submit again as chat or voice. If you're sure, " + "submit again as browser. Decide this from the source; don't ask anyone." +) +_FOUND_CHAT = ( + "second-look: you recorded modality {claimed}, and the source uses playwright. If that's a " + "helper and a person comes to this agent for a conversation, submit again as is. If its main " + "job is carrying out work on websites or a computer, submit again as browser, which stops " + "this build as not supported. Decide this from the source; don't ask anyone." +) +_FOUND_BROWSER = ( + "second-look: you recorded modality browser, which stops this build as not supported, and " + "the source uses playwright. A chat or voice agent can list one for its tests or use one in " + "a helper script. Does a person come to this agent for a conversation, or to have it carry " + "out work on websites or a computer? For a conversation, submit again as chat or voice. If " + "its main job is that work, submit again as browser. Decide this from the source; don't ask " + "anyone." +) +_NOTHING_FOUND_BROWSER = ( + "second-look: you recorded modality browser, which stops this build as not supported, and " + "no browser, crawling or computer-use tool was found in the source. Does a person come to " + "this agent for a conversation, or to have it carry out work on websites or a computer? For " + "a conversation, submit again as chat or voice. If you're sure, submit again as browser. " + "Decide this from the source; don't ask anyone." +) +_FOUND_MESSAGE = ( + "This agent's main job is driving a browser, operating a computer or crawling sites " + "(found: playwright). Only voice and chat agents are supported for now." +) +_READ_MESSAGE = ( + "This agent's main job was read as driving a browser, operating a computer or crawling " + "sites. Only voice and chat agents are supported for now." +) +# Over the size a .py is read at, so never read. +_OVERSIZED = "#" * (LARGEST_PARSED_MODULE + 1) + + +@pytest.fixture(autouse=True) +def _nothing_reaches_a_network(monkeypatch): + for name in ( + "FI_BASE_URL", + "OTEL_EXPORTER_OTLP_ENDPOINT", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + ): + monkeypatch.setenv(name, "http://127.0.0.1:9") + monkeypatch.delenv("FI_API_KEY", raising=False) + monkeypatch.delenv("FI_SECRET_KEY", raising=False) + + +@pytest.fixture(autouse=True) +def _a_known_receipt(monkeypatch): + monkeypatch.setattr(tools, "_new_receipt", lambda: _READ) + + +def _tree(root: Path, files: dict[str, str | bytes]) -> Path: + for name, body in files.items(): + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + if isinstance(body, bytes): + path.write_bytes(body) + else: + path.write_text(body, encoding="utf-8") + return root + + +def _reading(modality: str | None = "chat", **fields): + payload = { + "agent": "fixture", + "one_liner": "Answers questions.", + "real_use_cases": ["Answer a question"], + "tools": [{"name": "lookup", "args": ["query"]}], + "hard_constraints": ["Stay polite"], + "system_prompt_excerpt": "You answer questions.", + **fields, + } + if modality is not None: + payload["modality"] = modality + return payload + + +def _submitter(destination: Path, scan_root: Path, subfolder: str | None = None): + server = contract_tools(destination, scan_root=scan_root, subfolder=subfolder) + tool = next(one for one in server.tools if one.name == "submit_contract") + return lambda payload: asyncio.run(tool.handler(payload)) + + +def _text(answer) -> str: + return answer["content"][0]["text"] + + +def _look(text: str) -> str: + return _NOT_ACCEPTED + text + _RECEIPT + + +def _refused_with(output: Path) -> str: + assert not (output / "contract.json").exists() + return json.loads((output / FAILURE_FILE).read_text(encoding="utf-8"))["message"] + + +def _raise_for_agent(monkeypatch): + real = agent_type_check.read_module + + def odd(path, *args, **kwargs): + if path.name == "agent.py": + raise LookupError("an error no reader expects") + return real(path, *args, **kwargs) + + monkeypatch.setattr(agent_type_check, "read_module", odd) + + +# Totals set low, so a few bytes reach each limit as a large repository would. +_LIMITS = [ + pytest.param( + "file_size", + {"agent.py": "import playwright\n".ljust(LARGEST_PARSED_MODULE + 1, "#")}, + {}, + id="file_size-python", + ), + pytest.param( + "file_size", + {"agent.js": "const tool = 'computer-use-preview'; // past forty bytes\n"}, + {(agent_type_check, "_LARGEST_CODE_FILE"): 40}, + id="file_size-script", + ), + pytest.param( + "scan_total", + {"agent.py": "import playwright\n"}, + {(agent_type_check, "_LARGEST_SCAN"): 10}, + id="scan_total", + ), + pytest.param( + "script_total", + {"agent.js": "const tool = 'computer-use-preview'\n"}, + {(agent_type_check, "_LARGEST_SCRIPT_SCAN"): 10}, + id="script_total", + ), + pytest.param( + "python_total", + {"agent.py": "import playwright\n"}, + {(agent_type_check, "_LARGEST_PYTHON_SCAN"): 10}, + id="python_total", + ), + pytest.param( + "parse_total", + {"agent.py": "import playwright\n"}, + {(python_source, "LARGEST_PARSED_TOTAL"): 10}, + id="parse_total", + ), + pytest.param( + "parse_failed", + {"agent.py": "import playwright\ndef (\n"}, + {}, + id="parse_failed", + ), + pytest.param( + "token_total", + {"agent.py": "TOOL = 'computer_use_preview'\n"}, + {(python_source, "LARGEST_TOKENIZED_TOTAL"): 10}, + id="token_total-total", + ), + pytest.param( + "token_total", + {"agent.py": "TOOL = 'computer_use_preview'\n"}, + {(python_source, "LARGEST_TOKENIZED_WORK"): 1}, + id="token_total-work", + ), + pytest.param( + "decode_total", + {"agent.py": b"# coding: utf-7\nimport playwright\n"}, + {(python_source, "LARGEST_ESCAPED_TOTAL"): 0}, + id="decode_total", + ), + pytest.param( + "search_total", + {"agent.py": b"# coding: idna\nimport playwright\n"}, + {(agent_type_check, "_LARGEST_RAW_SEARCH"): 0}, + id="search_total", + ), + pytest.param( + "skipped", {"agent.py": "import playwright\n"}, _raise_for_agent, id="skipped" + ), + pytest.param( + "package_unreadable", + {"package.json": b'\xff{"dependencies": {"playwright": "1"}}'}, + {}, + id="package_unreadable", + ), +] + + +@pytest.mark.parametrize("limit, files, totals", _LIMITS) +def test_each_limit_cuts_the_scan(tmp_path, monkeypatch, limit, files, totals): + _tree(tmp_path, files) + if callable(totals): + totals(monkeypatch) + else: + for (module, name), value in totals.items(): + monkeypatch.setattr(module, name, value) + names, cut = browser_scan(tmp_path) + assert cut == frozenset({limit}) + assert decide("chat", names, looked_again=False, cut=cut) == "second_look" + + +@pytest.mark.parametrize( + "files, spent, names", + [ + # Read and charged, the padding would spend the total before the file that imports. + pytest.param( + { + **{f"a_pad_{at}.py": "x = 1\n" * (126 * 1024 // 6) for at in range(3)}, + "z_agent.py": "import playwright\n", + }, + False, + ["playwright"], + id="padding", + ), + pytest.param({"helper.py": "x = 1\n"}, True, [], id="needle-free"), + pytest.param( + {"agent.py": b"import playwright\n\xff\n"}, True, [], id="undecodable" + ), + ], +) +def test_a_file_that_cant_matter_is_no_cut(tmp_path, monkeypatch, files, spent, names): + _tree(tmp_path, files) + if spent: + monkeypatch.setattr(python_source, "LARGEST_PARSED_TOTAL", 0) + assert browser_scan(tmp_path) == (names, frozenset()) + + +@pytest.mark.parametrize("claimed", ["chat", "voice", "", "browser"]) +@pytest.mark.parametrize("found", [[], ["playwright"]]) +@pytest.mark.parametrize("looked_again", [False, True]) +def test_nothing_cut_is_todays_verdict_and_text(claimed, found, looked_again): + browser = claimed == "browser" + if browser: + today = "refuse" if looked_again else "second_look" + else: + today = "second_look" if found and not looked_again else "keep" + assert decide(claimed, found, looked_again=looked_again) == today + assert decide(claimed, found, looked_again=looked_again, cut=frozenset()) == today + if today != "second_look": + return + if browser: + text = _FOUND_BROWSER if found else _NOTHING_FOUND_BROWSER + else: + text = _FOUND_CHAT.format(claimed=claimed or "chat") + assert SecondLook.prompt(claimed, found) == text + assert SecondLook.prompt(claimed, found, cut=frozenset()) == text + assert SecondLook().issue(claimed, found, _READ) == text + _RECEIPT + + +@pytest.mark.parametrize( + "files, claimed, text", + [ + ( + {"agent.py": "import playwright\n"}, + "chat", + _FOUND_CHAT.format(claimed="chat"), + ), + ({"agent.py": "import playwright\n"}, "browser", _FOUND_BROWSER), + ({"agent.py": "import openai\n"}, "browser", _NOTHING_FOUND_BROWSER), + ], +) +def test_an_uncut_scan_gives_todays_look(tmp_path, files, claimed, text): + repo = _tree(tmp_path / "source", files) + assert browser_scan(repo)[1] == frozenset() + assert _text(_submitter(tmp_path / "out", repo)(_reading(claimed))) == _look(text) + + +def test_an_uncut_chat_reading_of_plain_code_is_saved_without_a_look(tmp_path): + repo = _tree(tmp_path / "source", {"agent.py": "import openai\n"}) + answer = _submitter(tmp_path / "out", repo)(_reading("chat")) + assert not answer.get("is_error"), answer + assert load(tmp_path / "out").modality == "chat" + + +def _cut_repo(root: Path, **more: str) -> Path: + return _tree(root, {"agent.py": "import openai\n", "big.py": _OVERSIZED, **more}) + + +@pytest.mark.parametrize( + "steps", + [ + pytest.param([("chat", None, "look"), ("chat", _READ, "saved")], id="chat"), + pytest.param([("voice", None, "look"), ("voice", _READ, "saved")], id="voice"), + pytest.param([(None, None, "look"), (None, _READ, "saved")], id="no-modality"), + pytest.param( + [("chat", None, "look"), ("browser", _READ, "refused")], id="then-browser" + ), + pytest.param( + [ + ("web", None, "problem"), + ("chat", None, "look"), + ("chat", _READ, "saved"), + ], + id="unknown-modality-first", + ), + ], +) +def test_a_cut_chat_reading_gets_one_look(tmp_path, steps): + output = tmp_path / "out" + submit = _submitter(output, _cut_repo(tmp_path / "source")) + for modality, receipt, then in steps: + extra = {} if receipt is None else {"second_look_read": receipt} + answer = submit(_reading(modality, **extra)) + if then == "problem": + assert answer["is_error"] + assert "'web' is not a modality" in _text(answer) + elif then == "look": + assert _text(answer) == _look(_CUT_CHAT.format(claimed=modality or "chat")) + elif then == "saved": + assert not answer.get("is_error"), answer + assert load(output).modality == (modality or "chat") + assert not (output / FAILURE_FILE).exists() + else: + assert _refused_with(output) == _READ_MESSAGE + + +def test_a_cut_reading_gets_a_look_on_the_subfolder_route(tmp_path): + source = tmp_path / "source" + _cut_repo(source / "agents" / "a") + output = tmp_path / "out" + submit = _submitter(output, source, "agents/a") + answer = submit(_reading("chat")) + assert _text(answer) == _look(_CUT_CHAT.format(claimed="chat")) + + +def test_a_package_json_that_isnt_a_file_is_no_cut(tmp_path): + repo = _tree(tmp_path / "source", {"agent.py": "import openai\n"}) + (repo / "package.json").symlink_to(repo / "nowhere.json") + assert browser_scan(repo) == ([], frozenset()) + + +@pytest.mark.parametrize( + "claimed, text, then, outcome", + [ + ("chat", _FOUND_CHAT.format(claimed="chat"), "chat", "saved"), + ("chat", _FOUND_CHAT.format(claimed="chat"), "browser", "refused"), + ("browser", _FOUND_BROWSER, "chat", "saved"), + ("browser", _FOUND_BROWSER, "browser", "refused"), + ], + ids=[ + "chat-then-chat", + "chat-then-browser", + "browser-then-chat", + "browser-then-browser", + ], +) +def test_cut_and_found_join_in_one_look(tmp_path, claimed, text, then, outcome): + repo = _cut_repo(tmp_path / "source", **{"agent.py": "import playwright\n"}) + assert browser_scan(repo) == (["playwright"], frozenset({"file_size"})) + assert ( + SecondLook.prompt(claimed, ["playwright"], cut=frozenset({"file_size"})) == text + ) + output = tmp_path / "out" + submit = _submitter(output, repo) + assert _text(submit(_reading(claimed))) == _look(text) + answer = submit(_reading(then, second_look_read=_READ)) + if outcome == "saved": + assert not answer.get("is_error"), answer + assert load(output).modality == then + else: + assert _refused_with(output) == _FOUND_MESSAGE + + +@pytest.mark.parametrize("then, outcome", [("browser", "refused"), ("chat", "saved")]) +def test_a_cut_browser_claim_drops_nothing_found(tmp_path, then, outcome): + output = tmp_path / "out" + submit = _submitter(output, _cut_repo(tmp_path / "source")) + first = _text(submit(_reading("browser"))) + assert first == _look(_CUT_BROWSER) + assert "was found" not in first + answer = submit(_reading(then, second_look_read=_READ)) + if outcome == "saved": + assert not answer.get("is_error"), answer + assert load(output).modality == "chat" + else: + assert _refused_with(output) == _READ_MESSAGE + + +@pytest.mark.parametrize( + "files, totals, said", + [ + pytest.param( + { + "big.py": _OVERSIZED, + "big_too.py": _OVERSIZED, + "agent.py": "import playwright\ndef (\n", + }, + {}, + "agent type scan cut by file_size 2, parse_failed 1", + id="two-limits", + ), + # A break leaves every file after it unread too, the one that broke it among them. + pytest.param( + {f"m{at}.py": "import playwright\n" for at in range(4)}, + {(agent_type_check, "_LARGEST_SCAN"): 30}, + "agent type scan cut by scan_total 3", + id="scan_total", + ), + pytest.param( + {f"m{at}.py": b"# coding: utf-7\nimport playwright\n" for at in range(2)}, + {(python_source, "LARGEST_ESCAPED_TOTAL"): 0}, + "agent type scan cut by decode_total 1", + id="decode_total", + ), + pytest.param({"agent.py": "import playwright\n"}, {}, None, id="uncut"), + ], +) +def test_the_cut_is_logged_once(tmp_path, monkeypatch, caplog, files, totals, said): + _tree(tmp_path, files) + for (module, name), value in totals.items(): + monkeypatch.setattr(module, name, value) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + browser_scan(tmp_path) + browser_scan(tmp_path) + browser_tools_found(tmp_path) + first = [(record.levelno, record.getMessage()) for record in caplog.records] + caplog.clear() + agent_type_check._scan.cache_clear() + browser_scan(tmp_path) + again = [(record.levelno, record.getMessage()) for record in caplog.records] + assert first == again == ([] if said is None else [(logging.WARNING, said)]) + + +def _shared_repo(root: Path) -> Path: + return _tree( + root, + { + "agents/a/agent.py": "import common.web\n", + "agents/a/big.py": _OVERSIZED, + "common/__init__.py": "", + "common/web.py": "import playwright\n", + }, + ) + + +def test_one_call_walks_once(tmp_path, monkeypatch): + repo = _shared_repo(tmp_path / "source") + walks: list[str] = [] + scans: list[int] = [] + real_walk, real_scan = ( + agent_type_check.imported_by_agent, + agent_type_check._tools_in, + ) + + def walk(root, subfolder, **kwargs): + walks.append(subfolder) + return real_walk(root, subfolder, **kwargs) + + def scan(*args, **kwargs): + scans.append(1) + return real_scan(*args, **kwargs) + + monkeypatch.setattr(agent_type_check, "imported_by_agent", walk) + monkeypatch.setattr(agent_type_check, "_tools_in", scan) + reached: list[Path] = [] + assert browser_scan(repo, "agents/a", reached) == ( + ["playwright"], + frozenset({"file_size"}), + ) + assert walks == ["agents/a"] + assert len(scans) == 1 + assert repo.resolve() / "common" / "web.py" in reached + assert browser_tools_in_agent(repo, "agents/a") == ["playwright"] + + agent_type_check._scan.cache_clear() + scans.clear() + assert browser_scan(repo) == (["playwright"], frozenset({"file_size"})) + assert browser_scan(repo) == (["playwright"], frozenset({"file_size"})) + assert browser_tools_found(repo) == ["playwright"] + assert len(scans) == 1 + assert agent_type_check._scan.cache_info().currsize == 1 + + +def test_no_root_is_no_scan(): + assert browser_scan(None) == ([], frozenset()) + + +def test_the_tickets_padding_gets_a_look(tmp_path): + # Three 126 KB files naming playwright only in a comment spend the parse total first. + pad = "# playwright\n" + "# padding\n" * (126 * 1024 // 10) + repo = _tree( + tmp_path / "source", + { + **{f"a_pad_{at}.py": pad for at in range(3)}, + "z_agent.py": "import playwright\n", + }, + ) + assert browser_scan(repo) == ([], frozenset({"parse_total"})) + answer = _submitter(tmp_path / "out", repo)(_reading("chat")) + assert _text(answer) == _look(_CUT_CHAT.format(claimed="chat")) + + +def test_a_large_honest_repository_gets_a_look(tmp_path, monkeypatch): + # Twenty 4 KB modules read before the agent's main file, against a total of 64 KB standing in + # for the 128 MiB one a 118 MB repository runs out of. + module = "".join(f"def f{at}():\n return {at}\n" for at in range(200)) + repo = _tree( + tmp_path / "source", + { + **{f"lib/mod_{at:02}.py": module for at in range(20)}, + "service/main.py": "from playwright.sync_api import sync_playwright\n", + }, + ) + assert browser_scan(repo) == (["playwright"], frozenset()) + monkeypatch.setattr(agent_type_check, "_LARGEST_SCAN", 64 * 1024) + agent_type_check._scan.cache_clear() + assert browser_scan(repo) == ([], frozenset({"scan_total"})) + answer = _submitter(tmp_path / "out", repo)(_reading("chat")) + assert _text(answer) == _look(_CUT_CHAT.format(claimed="chat")) + + +def test_the_walks_stop_is_no_cut(tmp_path, caplog): + # Over the walk's 256 KB parse total in modules that import, none of them naming a tool. + module = "import os\n" + "x = 1\n" * (100 * 1024 // 6) + repo = _tree( + tmp_path / "source", + { + **{f"agents/a/m_{at}.py": module for at in range(3)}, + "agents/a/z_main.py": "import common.util\n", + "common/__init__.py": "", + "common/util.py": "x = 1\n", + }, + ) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + assert browser_scan(repo, "agents/a") == ([], frozenset()) + assert any( + "stopped at its limits" in record.getMessage() for record in caplog.records + ) + output = tmp_path / "out" + answer = _submitter(output, repo, "agents/a")(_reading("chat")) + assert not answer.get("is_error"), answer + assert load(output).modality == "chat" + + +def test_ci_runs_this_file(): + workflows = ( + Path(__file__).resolve().parents[2] / ".github" / "workflows" / "sdk-smoke.yml" + ) + assert "tests/harness/test_browser_scan_cut.py" in workflows.read_text( + encoding="utf-8" + ) diff --git a/tests/harness/test_contract_tools_subfolder.py b/tests/harness/test_contract_tools_subfolder.py index 467a5f622..fcdc7db08 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2,6 +2,7 @@ import asyncio import json +import re import sys from pathlib import Path @@ -1043,13 +1044,20 @@ async def beat(): worst[0], last = max(worst[0], now - last), now beating = asyncio.create_task(beat()) - result = await submit(_plain("crafted")) + # The scan doesn't read the crafted file, and says so, so the reading gets a look first. + look = await submit(_plain("crafted")) + receipt = re.search(r'second_look_read to "([^"]+)"', look["content"][0]["text"]) + assert receipt, look + result = await submit({**_plain("crafted"), "second_look_read": receipt[1]}) done.set() await beating - return result, worst[0] + return look, result, worst[0] - result, worst = asyncio.run(save()) + look, result, worst = asyncio.run(save()) + assert look.get("is_error") + assert "second-look" in look["content"][0]["text"] + assert "stopped before reading" in look["content"][0]["text"] assert not result.get("is_error"), result assert _saved(destination)["instrumentation"] == "traceai" assert worst < 0.25 From 7722f1b522ade8fcab0581532fb3133908800b30 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 06:12:55 +0530 Subject: [PATCH 2/8] test(harness): put the walk-stop row over TH-9543's 512 KB walk total (TH-9512) Signed-off-by: khushalsonawat --- tests/harness/test_browser_scan_cut.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index 4b8961545..5b6f7386c 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -557,12 +557,12 @@ def test_a_large_honest_repository_gets_a_look(tmp_path, monkeypatch): def test_the_walks_stop_is_no_cut(tmp_path, caplog): - # Over the walk's 256 KB parse total in modules that import, none of them naming a tool. + # Over the walk's 512 KB parse total in modules that import, none of them naming a tool. module = "import os\n" + "x = 1\n" * (100 * 1024 // 6) repo = _tree( tmp_path / "source", { - **{f"agents/a/m_{at}.py": module for at in range(3)}, + **{f"agents/a/m_{at}.py": module for at in range(6)}, "agents/a/z_main.py": "import common.util\n", "common/__init__.py": "", "common/util.py": "x = 1\n", From b71f86cf8008626e239f6f3443cb22c182394cbe Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 16:37:45 +0530 Subject: [PATCH 3/8] fix(harness): cut only an oversize file that may name a browser tool; read package.json as Node does (TH-9512) - An oversize .py counts as file_size only when its bytes name a browser package, its coding line names a codec that isn't raw-safe or that Python refuses, or the search can't clear it: at most 4 MiB a file and 8 MiB a scan are searched, and a file past either bound, unreadable, not a file, or grown since its size was taken still cuts. It is never parsed. - package.json is read as Node reads it, UTF-8 with replacement characters and integers kept as text; invalid JSON means no dependencies, since npm refuses it too. Only OSError and RecursionError leave it unreadable, a cut. TH-9512 Signed-off-by: khushalsonawat --- src/fi/alk/harness/agent_type_check.py | 79 ++++++- tests/harness/test_agent_type_refusal.py | 2 +- tests/harness/test_browser_scan_cut.py | 221 +++++++++++++++++- .../harness/test_contract_tools_subfolder.py | 16 +- 4 files changed, 297 insertions(+), 21 deletions(-) diff --git a/src/fi/alk/harness/agent_type_check.py b/src/fi/alk/harness/agent_type_check.py index 5c5a95040..d35506aa9 100644 --- a/src/fi/alk/harness/agent_type_check.py +++ b/src/fi/alk/harness/agent_type_check.py @@ -26,10 +26,12 @@ from .instrumentation_evidence import _SKIPPED_DIRECTORIES, _imports from .job import FailureDomain, HarnessFailure, HarnessStage from .python_source import ( + _RAW_SAFE_CODECS, LARGEST_PARSED_MODULE, EscapeBudget, ParseBudget, TokenBudget, + _coding, parsed, read_module, ) @@ -119,6 +121,7 @@ _MARKER_START = re.compile( r"computer[_-]?[uU]se|ComputerUse|computer_\d{8}|@playwright/mcp|mcp-server-playwright" ) +_MARKER_START_BYTES = re.compile(_MARKER_START.pattern.encode()) _TOOLKIT = "PlayWrightBrowserToolkit" # A whole name: no letter, digit or "_" before or after it. In a unicode-escape file read as bytes, # an escape such as \t, \x20, \040 or \u0020 just before it counts as a boundary too. @@ -144,6 +147,16 @@ _LARGEST_CODE_FILE = 1024 * 1024 # What one scan searches with regular expressions in files it can't decode (see _tools_in). _LARGEST_RAW_SEARCH = 1024 * 1024 +# A file too big to read is searched as bytes for the names that would make it matter (see +# _may_name_a_tool), up to this size: a generated or data module of a few MB is cleared. Larger +# ones aren't read at all. +_LARGEST_UNREAD_SEARCHED = 4 * 1024 * 1024 +# And what one scan searches of them in all. Every name is looked for in each file: about 18 ms a +# MiB on 3.12.3, crafted near misses included, so this much is about 0.15 s. A coding line naming a +# codec costs more: the lookup normalises the name in Python, so two crafted 4 MiB names took +# 0.31 s, and encodings._cache keeps each name it misses, 8 M characters for those two, for as long +# as the process runs. read_module looks up the names of files it reads the same way. +_UNREAD_SEARCH_TOTAL = 8 * 1024 * 1024 # Past this a repository holds the conversation up for longer than the rest of the scan is worth. _LARGEST_SCAN = 128 * 1024 * 1024 # Scripts are read by regular expressions that call back for every string or comment: 128 MB of @@ -402,15 +415,24 @@ def _agent_files( def _packages_used(path: Path) -> set[str] | None: - """The browser packages ``path`` lists, or None when it can't be read: npm may still read it.""" + """The browser packages ``path`` lists, or None when it can't be read: npm may still read it. + + Read as Node reads it, a byte that isn't UTF-8 becoming a replacement character. Text that + isn't JSON lists nothing: npm refuses it too. + """ try: # A link to nothing or a pipe isn't a manifest npm reads either. if not path.is_file(): return set() # Windows editors often save a BOM, which json refuses. - manifest = json.loads(path.read_text(encoding="utf-8").removeprefix("\ufeff")) - except (OSError, ValueError, RecursionError): + text = path.read_text(encoding="utf-8", errors="replace").removeprefix("\ufeff") + # JSON.parse reads an integer of any length, which json refuses past 4,300 digits; the + # scan uses no numbers, so they're kept as text. + manifest = json.loads(text, parse_int=str) + except (OSError, RecursionError): return None + except ValueError: + return set() if not isinstance(manifest, dict): return set() names: set[str] = set() @@ -521,6 +543,46 @@ def _file_size(path: Path) -> int: return 0 +def _may_name_a_tool(path: Path, size: int, left: list[int]) -> bool: + """Whether a file too big to read, of ``size`` bytes, may name a tool, so leaving it unread + cuts the scan. + + Its bytes are searched for the names a Python file must hold to be read at all + (``_PYTHON_NAMES``, as ``read_module`` searches them), "omputer" aside: that is a word in + prose, so it counts only where a ``_MARKER_START`` match starts. Every such match holds + "omputer" or "playwright", so this covers a script's tool markers too. It's never decoded or + parsed. A Python file whose coding line names a codec not proven safe to search as bytes can + spell a name its bytes don't show, so it may; so may one whose coding line Python refuses, + since the codec can be registered at run time before the file is imported. A file that can't + be cleared may: one over ``_LARGEST_UNREAD_SEARCHED``, over what is ``left`` of the scan's + ``_UNREAD_SEARCH_TOTAL``, one that can't be read, or a path that isn't a regular file. So may + a ``package.json``: the packages it lists aren't among these names. + """ + if path.name == "package.json": + return True + if size > _LARGEST_UNREAD_SEARCHED or size > left[0]: + return True + try: + # A link can name a device or a pipe, which reads without end. + if not path.is_file(): + return True + with path.open("rb") as handle: + raw = handle.read(_LARGEST_UNREAD_SEARCHED + 1) + except (OSError, ValueError): + return True + left[0] -= len(raw) + # It may have grown since its size was taken. + if len(raw) > _LARGEST_UNREAD_SEARCHED or left[0] < 0: + return True + if path.suffix == ".py": + coding = _coding(raw) + if coding is None or coding[1].name not in _RAW_SAFE_CODECS: + return True + if any(name in raw for name in _PYTHON_NAMES if name != b"omputer"): + return True + return b"omputer" in raw and _MARKER_START_BYTES.search(raw) is not None + + def browser_tools_found(source_root: Path | None) -> list[str]: """The browser, crawling and computer-use tools the agent's own code uses, by display name. @@ -612,7 +674,9 @@ def _tools_in( And the limits that cut the scan, by name: each left code unread or unparsed, so finding nothing then doesn't say there's nothing, and the reading gets a second look. A file that - names no tool, or that Python can't decode, isn't one: it can't import what Python runs. + names no tool, or that Python can't decode, isn't one: it can't import what Python runs. A + file too big to read is searched as bytes to say whether it names one (``_may_name_a_tool``), + so a large data module that doesn't isn't one either. """ found: set[str] = set() imported: set[str] = set() @@ -623,15 +687,18 @@ def _tools_in( tokens = TokenBudget() escapes = EscapeBudget() searched = ParseBudget(_LARGEST_RAW_SEARCH) + unread = [_UNREAD_SEARCH_TOTAL] ordered = sorted( filter(_is_read, paths), key=lambda path: (first is not None and not is_within(path, first), *_reading_order(path)), ) for at, path in enumerate(ordered): size = _file_size(path) - # Never read, so it mustn't use up the limit before the code that is. + # Never read as code, so it mustn't use up the limits before the code that is: what its + # search reads has its own total. if size > _largest_read(path): - cut["file_size"] += 1 + if _may_name_a_tool(path, size, unread): + cut["file_size"] += 1 continue scanned += size if scanned > _LARGEST_SCAN: diff --git a/tests/harness/test_agent_type_refusal.py b/tests/harness/test_agent_type_refusal.py index dba66901c..4e74e1555 100644 --- a/tests/harness/test_agent_type_refusal.py +++ b/tests/harness/test_agent_type_refusal.py @@ -667,7 +667,7 @@ def test_a_nul_byte_the_tokenizer_breaks_on_reaches_no_warning(tmp_path, caplog) with caplog.at_level("DEBUG", logger=agent_type_check.__name__): assert browser_tools_found(tmp_path) == ["playwright"] # The tokenizer's fault reaches no WARNING of its own. The files name a tool and can't be - # parsed, so they cut the scan (TH-9512), and that line holds names and counts only. + # parsed, so they cut the scan, and that line holds names and counts only. loud = [record.getMessage() for record in caplog.records if record.levelno >= 20] assert loud == ["agent type scan cut by parse_failed 30"] assert "SOURCE-LINE" not in caplog.text diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index 5b6f7386c..0b6ae8f73 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -9,6 +9,7 @@ import asyncio import json import logging +import os from pathlib import Path import pytest @@ -76,8 +77,12 @@ "This agent's main job was read as driving a browser, operating a computer or crawling " "sites. Only voice and chat agents are supported for now." ) -# Over the size a .py is read at, so never read. -_OVERSIZED = "#" * (LARGEST_PARSED_MODULE + 1) +# Over the size a .py is read at, so never read; it names a tool, so leaving it unread cuts. +_OVERSIZED = "# playwright\n".ljust(LARGEST_PARSED_MODULE + 1, "#") +# A data module of prompt strings over that size, about 265 KB, naming no tool. +_DATA_MODULE = ( + "PROMPTS = [\n" + ' "Tell me about the weather today.",\n' * 6_800 + "]\n" +) @pytest.fixture(autouse=True) @@ -226,7 +231,7 @@ def odd(path, *args, **kwargs): ), pytest.param( "package_unreadable", - {"package.json": b'\xff{"dependencies": {"playwright": "1"}}'}, + {"package.json": "[" * 100_000}, {}, id="package_unreadable", ), @@ -376,6 +381,216 @@ def test_a_package_json_that_isnt_a_file_is_no_cut(tmp_path): assert browser_scan(repo) == ([], frozenset()) +def test_a_large_data_module_is_no_cut(tmp_path): + # A big data module beside code that imports no tool. + assert len(_DATA_MODULE.encode()) > 260 * 1024 + repo = _tree( + tmp_path / "a", {"main.py": "import openai\n", "prompts_data.py": _DATA_MODULE} + ) + assert browser_scan(repo) == ([], frozenset()) + output = tmp_path / "out" + answer = _submitter(output, repo)(_reading("chat")) + assert not answer.get("is_error"), answer + assert load(output).modality == "chat" + + +@pytest.mark.parametrize( + "files, totals", + [ + pytest.param( + {"big.py": "#" * (LARGEST_PARSED_MODULE + 1) + "\nimport playwright\n"}, + {}, + id="named-past-the-read-limit", + ), + pytest.param( + {"big.py": "#" * (3 * 1024 * 1024) + "\nimport playwright\n"}, + {}, + id="named-3-mib-in", + ), + # Python refuses the coding line until the codec is registered at run time. + pytest.param( + {"big.py": b"# coding: nonesuch\n" + _DATA_MODULE.encode()}, + {}, + id="refused-coding", + ), + pytest.param( + {"big.py": "x = 1\n" * 30_000 + "TOOL = 'computer_use_preview'\n"}, + {}, + id="marker", + ), + # Python reads it as `import playwright`; its bytes name nothing. + pytest.param( + { + "big.py": b"# coding: utf-7\nimport +AHA-laywright\n" + + _DATA_MODULE.encode() + }, + {}, + id="utf-7", + ), + pytest.param( + { + "big.py": b"# coding: unicode_escape\nimport \\x70laywright\n" + + _DATA_MODULE.encode() + }, + {}, + id="unicode-escape", + ), + pytest.param( + {"big.py": _DATA_MODULE}, + {(agent_type_check, "_LARGEST_UNREAD_SEARCHED"): LARGEST_PARSED_MODULE + 1}, + id="past-the-file-bound", + ), + pytest.param( + {"big.py": _DATA_MODULE}, + {(agent_type_check, "_UNREAD_SEARCH_TOTAL"): len(_DATA_MODULE) - 1}, + id="past-the-scan-total", + ), + pytest.param( + {"bundle.js": "const tool = 'computer-use-preview';\n"}, + {(agent_type_check, "_LARGEST_CODE_FILE"): 20}, + id="script-marker", + ), + # The packages a manifest lists aren't among the names searched for. + pytest.param( + {"package.json": json.dumps({"dependencies": {"puppeteer": "1"}})}, + {(agent_type_check, "_LARGEST_CODE_FILE"): 20}, + id="package-json", + ), + ], +) +def test_an_unread_file_that_may_name_a_tool_cuts(tmp_path, monkeypatch, files, totals): + for (module, name), value in totals.items(): + monkeypatch.setattr(module, name, value) + repo = _tree(tmp_path / "source", {"main.py": "import openai\n", **files}) + assert browser_scan(repo) == ([], frozenset({"file_size"})) + + +@pytest.mark.parametrize( + "files, totals", + [ + # "computer" in prose starts no tool marker. + pytest.param( + {"big.py": _DATA_MODULE.replace("weather today", "computer", 1)}, + {}, + id="prose-computer", + ), + pytest.param( + {"big.py": b"# coding: latin-1\n" + _DATA_MODULE.encode()}, + {}, + id="raw-safe-coding", + ), + pytest.param( + {"bundle.js": "const answer = 42; // past twenty bytes\n"}, + {(agent_type_check, "_LARGEST_CODE_FILE"): 20}, + id="script", + ), + # The second file's search spends what's left exactly. + pytest.param( + {"big.py": _DATA_MODULE, "big_too.py": _DATA_MODULE}, + {(agent_type_check, "_UNREAD_SEARCH_TOTAL"): 2 * len(_DATA_MODULE)}, + id="at-the-scan-total", + ), + ], +) +def test_an_unread_file_that_names_no_tool_is_no_cut( + tmp_path, monkeypatch, files, totals +): + for (module, name), value in totals.items(): + monkeypatch.setattr(module, name, value) + repo = _tree(tmp_path / "source", {"main.py": "import openai\n", **files}) + assert browser_scan(repo) == ([], frozenset()) + + +def _unreadable(monkeypatch, repo: Path) -> None: + real = Path.open + + def refused(self, *args, **kwargs): + if self.name == "big.py": + raise PermissionError("refused") + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "open", refused) + + +def _sized(size: int): + real = agent_type_check._file_size + return lambda path: size if path.name == "big.py" else real(path) + + +def _not_a_file(monkeypatch, repo: Path) -> None: + # Read, it would give nothing, and nothing names no tool. + (repo / "big.py").unlink() + (repo / "big.py").symlink_to(os.devnull) + monkeypatch.setattr( + agent_type_check, "_file_size", _sized(LARGEST_PARSED_MODULE + 1) + ) + + +def _grown(monkeypatch, repo: Path) -> None: + # Its size taken under the bound, it's past it when read. + (repo / "big.py").write_text("x = 1\n" * (5 * 1024 * 1024 // 6), encoding="utf-8") + monkeypatch.setattr( + agent_type_check, "_file_size", _sized(LARGEST_PARSED_MODULE + 1) + ) + + +@pytest.mark.parametrize("make", [_unreadable, _not_a_file, _grown]) +def test_an_unread_file_that_cant_be_cleared_cuts(tmp_path, monkeypatch, make): + repo = _tree( + tmp_path / "source", {"main.py": "import openai\n", "big.py": _DATA_MODULE} + ) + make(monkeypatch, repo) + assert browser_scan(repo) == ([], frozenset({"file_size"})) + + +def test_the_unread_search_is_charged_to_its_own_total(tmp_path, monkeypatch): + # Read first by name: charged to the scan's totals, it would leave the agent's file unread. + repo = _tree( + tmp_path / "source", + {"a_data.py": _DATA_MODULE, "z_agent.py": "import playwright\n"}, + ) + monkeypatch.setattr(agent_type_check, "_LARGEST_SCAN", 100) + monkeypatch.setattr(agent_type_check, "_LARGEST_PYTHON_SCAN", 100) + assert browser_scan(repo) == (["playwright"], frozenset()) + # The search's total is shared by the scan's files, in the order they're read. + agent_type_check._scan.cache_clear() + _tree(repo, {"b_data.py": _DATA_MODULE}) + monkeypatch.setattr(agent_type_check, "_UNREAD_SEARCH_TOTAL", len(_DATA_MODULE)) + assert browser_scan(repo) == (["playwright"], frozenset({"file_size"})) + + +@pytest.mark.parametrize( + "manifest, names", + [ + # A trailing comma: npm refuses it too. + pytest.param( + b'{"name": "x", "dependencies": {"react": "1"},}', [], id="trailing-comma" + ), + # Node reads a byte that isn't UTF-8 as a replacement character. + pytest.param( + b'{"description": "\xff", "dependencies": {"playwright": "1"}}', + ["playwright"], + id="not-utf-8-in-a-string", + ), + # Read that way, a stray byte before the object is no JSON either. + pytest.param( + b'\xff{"dependencies": {"playwright": "1"}}', [], id="not-utf-8-before-it" + ), + # JSON.parse reads an integer of any length; json refuses one past 4,300 digits. + pytest.param( + b'{"dependencies": {"puppeteer": "22"}, "n": ' + b"1" * 5000 + b"}", + ["puppeteer"], + id="a-5000-digit-integer", + ), + ], +) +def test_a_package_json_npm_reads_or_refuses_is_no_cut(tmp_path, manifest, names): + repo = _tree( + tmp_path / "b", {"main.py": "import openai\n", "package.json": manifest} + ) + assert browser_scan(repo) == (names, frozenset()) + + @pytest.mark.parametrize( "claimed, text, then, outcome", [ diff --git a/tests/harness/test_contract_tools_subfolder.py b/tests/harness/test_contract_tools_subfolder.py index 04de8dc4b..b6e21fae1 100644 --- a/tests/harness/test_contract_tools_subfolder.py +++ b/tests/harness/test_contract_tools_subfolder.py @@ -2,7 +2,6 @@ import asyncio import json -import re import sys from pathlib import Path @@ -1044,20 +1043,15 @@ async def beat(): worst[0], last = max(worst[0], now - last), now beating = asyncio.create_task(beat()) - # The scan doesn't read the crafted file, and says so, so the reading gets a look first. - look = await submit(_plain("crafted")) - receipt = re.search(r'second_look_read to "([^"]+)"', look["content"][0]["text"]) - assert receipt, look - result = await submit({**_plain("crafted"), "second_look_read": receipt[1]}) + # The scan doesn't parse the crafted file; searched as bytes, it names no browser tool, so + # leaving it unread doesn't cut the scan, and the reading is saved without a look. + result = await submit(_plain("crafted")) done.set() await beating - return look, result, worst[0] + return result, worst[0] - look, result, worst = asyncio.run(save()) + result, worst = asyncio.run(save()) - assert look.get("is_error") - assert "second-look" in look["content"][0]["text"] - assert "stopped before reading" in look["content"][0]["text"] assert not result.get("is_error"), result assert _saved(destination)["instrumentation"] == "traceai" assert worst < 0.25 From da2fba5fd40318b4e6da68bb1e54f6363a0ac859 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 18:36:59 +0530 Subject: [PATCH 4/8] test(harness): pin the unread search's bounds and the toolkit name (TH-9512) - A file exactly at the per-file search bound, its needle in the last bytes, cuts. - An oversize module whose only tool is PlayWrightBrowserToolkit cuts. - Past what's left of the scan's search total, an oversize file isn't opened. TH-9512 Signed-off-by: khushalsonawat --- tests/harness/test_browser_scan_cut.py | 37 ++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index 0b6ae8f73..f755234cc 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -407,6 +407,24 @@ def test_a_large_data_module_is_no_cut(tmp_path): {}, id="named-3-mib-in", ), + # Exactly the per-file bound: the needle ends at its last byte. + pytest.param( + { + "big.py": "#" * (agent_type_check._LARGEST_UNREAD_SEARCHED - 19) + + "\nimport playwright\n" + }, + {}, + id="named-at-the-bound", + ), + # The toolkit's import holds no lowercase `playwright`. + pytest.param( + { + "big.py": "x = 1\n" * 30_000 + + "from langchain_community.agent_toolkits import PlayWrightBrowserToolkit\n" + }, + {}, + id="toolkit", + ), # Python refuses the coding line until the codec is registered at run time. pytest.param( {"big.py": b"# coding: nonesuch\n" + _DATA_MODULE.encode()}, @@ -465,6 +483,25 @@ def test_an_unread_file_that_may_name_a_tool_cuts(tmp_path, monkeypatch, files, assert browser_scan(repo) == ([], frozenset({"file_size"})) +def test_past_the_search_total_a_file_isnt_opened(tmp_path, monkeypatch): + repo = _tree( + tmp_path / "source", + {"main.py": "import openai\n", "big.py": _DATA_MODULE, "big_too.py": _DATA_MODULE}, + ) + monkeypatch.setattr(agent_type_check, "_UNREAD_SEARCH_TOTAL", len(_DATA_MODULE) - 1) + opened: list[str] = [] + real = Path.open + + def spy(self, *args, **kwargs): + opened.append(self.name) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "open", spy) + assert browser_scan(repo) == ([], frozenset({"file_size"})) + # Neither fits what's left of the total, so neither is read to be searched. + assert "big.py" not in opened and "big_too.py" not in opened + + @pytest.mark.parametrize( "files, totals", [ From 868b243efd2095fda56dcebef0263c2a56e6d006 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 15:03:19 +0530 Subject: [PATCH 5/8] fix(harness): follow a walk past its parse total with a line search, and cut only when that stops too (TH-9512) A file the walk reached but its parse total, parse size or script total kept from being parsed has its import lines searched (no parse) and followed under the other limits, with one 8 MiB search total for the walk. The scan's cut set gets import_walk only when a limit or that search's cap left code unread; a file the search covered is no cut. The WARNING stays. Tests pin Dileep's repro, a two-hop line-search chain, and the search cap. Signed-off-by: khushalsonawat --- src/fi/alk/harness/agent_type_check.py | 28 +++-- src/fi/alk/harness/credentials.py | 145 ++++++++++++++++++++--- tests/harness/test_agent_type_refusal.py | 70 ++++++++--- tests/harness/test_browser_scan_cut.py | 85 +++++++++++-- 4 files changed, 280 insertions(+), 48 deletions(-) diff --git a/src/fi/alk/harness/agent_type_check.py b/src/fi/alk/harness/agent_type_check.py index b1814574d..ac23723b3 100644 --- a/src/fi/alk/harness/agent_type_check.py +++ b/src/fi/alk/harness/agent_type_check.py @@ -178,6 +178,10 @@ # And the lookups on disk it makes then, a file found costing one for each part of its path: a name # can resolve a package for each of its parts, and 1,000 crafted names took the save 12 s. _WALKED_LOOKUPS = 100_000 +# And the import lines it searches then, in the files its parse total keeps from being parsed, in +# all: the size of the unread-file search (see _UNREAD_SEARCH_TOTAL). Past it, the code left +# unread is a cut. +_WALKED_SEARCHED = _UNREAD_SEARCH_TOTAL # Per scan, each record read once; a reopen of unchanged files reads none. Each row is parsed and # kept, so short rows cost the most: a scan through 16 MiB of 7-byte rows takes 0.9 s. A whole # 411-package environment unpacked in one folder came to 6.4 MiB. @@ -808,11 +812,14 @@ def _below_shared(path: Path, folder: Path) -> tuple[str, ...]: return held[common:] -def _walked(root: Path, subfolder: str) -> frozenset[Path]: - """What the agent's folder imports, followed with the save's limits. A walk a limit stopped +def _walked(root: Path, subfolder: str) -> tuple[frozenset[Path], bool]: + """What the agent's folder imports, followed with the save's limits, and whether a budget or + limit left code it reached unread (the scan's ``import_walk`` cut). A walk a limit stopped is logged: what it didn't reach, the save's checks don't see, and bundling's walk, with no - limit, may.""" + limit, may. A stop for a name's length or depth, a link, a syntax error or a codec the + parser refuses is logged but isn't a cut.""" stopped: list[bool] = [] + limit_cut: list[bool] = [] found = imported_by_agent( root, subfolder, @@ -822,6 +829,8 @@ def _walked(root: Path, subfolder: str) -> frozenset[Path]: import_limit=_WALKED_IMPORTS, lookup_limit=_WALKED_LOOKUPS, stopped=stopped, + limit_cut=limit_cut, + search_total=_WALKED_SEARCHED, ) if stopped and stopped[0]: logger.warning( @@ -830,7 +839,7 @@ def _walked(root: Path, subfolder: str) -> frozenset[Path]: subfolder, len(found), ) - return found + return found, bool(limit_cut and limit_cut[0]) def code_the_agent_reaches(source_root: Path, subfolder: str) -> list[Path]: @@ -865,7 +874,8 @@ def _scanned_agent( source_root: Path, subfolder: str, reached: list[Path] | None ) -> tuple[tuple[str, ...], frozenset[str]]: """``browser_tools_in_agent``'s scan, and the limits that cut it. The walk's own stops aren't - among them: they're logged by ``_walked``.""" + among them, except ``import_walk`` when a budget or limit left code unread; the walk's other + stops are logged by ``_walked``.""" root = Path(source_root).resolve() folder = agent_folder(root, subfolder) above = [ @@ -904,7 +914,8 @@ def unpacked(path: Path) -> bool: break return False - walked = sorted(_walked(root, subfolder)) + reached_files, walk_cut = _walked(root, subfolder) + walked = sorted(reached_files) shared = [ path for path in walked @@ -934,9 +945,12 @@ def unpacked(path: Path) -> bool: for parent in path.parents if is_within(parent, root) and (parent / "package.json").is_file() ] - return _tools_in( + names, cut = _tools_in( dict.fromkeys((*_agent_files(folder), *above, *shared, *manifests)), local, folder ) + # A budget or limit left code the walk reached unread, so what it would have led to is + # unknown (TH-9512, Dileep's C2). _tools_in has already logged its own cuts. + return names, (cut | {"import_walk"} if walk_cut else cut) def modality_text(value: Any) -> str: diff --git a/src/fi/alk/harness/credentials.py b/src/fi/alk/harness/credentials.py index 1c11305d3..c8fc5d927 100644 --- a/src/fi/alk/harness/credentials.py +++ b/src/fi/alk/harness/credentials.py @@ -677,6 +677,7 @@ def _file_inside( deepest: int, left: list[float] | None = None, unread: list[bool] | None = None, + too_big: list[bool] | None = None, ) -> Path | None: """``candidate`` resolved, if it's a plain file in ``root`` at most ``deepest`` parts long. @@ -690,7 +691,7 @@ def _file_inside( return None if left is not None: left[0] -= 1 - if not _plain_file(candidate, unread): + if not _plain_file(candidate, unread, too_big): return None if left is not None: left[0] -= len(candidate.parts) @@ -1297,17 +1298,26 @@ def _read_in_place(real: Path, folder: Path) -> bool: ) -def _plain_file(path: Path, unread: list[bool] | None = None) -> bool: +def _plain_file( + path: Path, unread: list[bool] | None = None, too_big: list[bool] | None = None +) -> bool: """A regular file small enough to read: a committed link can point at anything, so it isn't followed. Given ``unread``, its one item is set when a code file is refused for its size or for being a link, since Python and Node run what one holds or points at. A dangling link is - marked too: telling it apart would follow its chain uncharged.""" + marked too: telling it apart would follow its chain uncharged. Given ``too_big``, its one item + is set for a regular code file over the size cap, which is a limit, where a link is not.""" try: details = path.lstat() except (OSError, ValueError): return False if stat.S_ISREG(details.st_mode) and details.st_size <= _MAX_FILE_BYTES: return True + if ( + too_big is not None + and stat.S_ISREG(details.st_mode) + and (path.suffix == ".py" or path.suffix in _SCRIPT_SUFFIXES) + ): + too_big[0] = True if (stat.S_ISREG(details.st_mode) or stat.S_ISLNK(details.st_mode)) and ( path.suffix == ".py" or path.suffix in _SCRIPT_SUFFIXES ): @@ -1339,6 +1349,48 @@ def _python_imports( return [(level, name) for level, name in names if name] +_IMPORT_LINE_NAME = re.compile(r"[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)*") +_FROM_LINE = re.compile(r"from[ \t]+(\.*)[ \t]*([A-Za-z_][\w.]*)?[ \t]+import\b(.*)", re.DOTALL) + + +def _import_lines(content: str) -> list[tuple[int, str]]: + """The names a line search finds ``content`` importing, each with its level, in the shape + ``_python_imports`` gives: ``import a.b, c`` and ``from .x import y`` / ``from a.b import c`` + at the start of a line (indented or not), whatever follows a ``#`` or a ``;`` ignored. A + parenthesised list is read across its lines. No parse, so no budget: the caller charges the + bytes to the walk's search total. + + It misses an import built at run time (``importlib.import_module(name)``, ``__import__``), one + inside a multi-line string or after a ``;``, and a name only a decoded codec spells. + """ + names: list[tuple[int, str]] = [] + lines = iter(content.split("\n")) + for line in lines: + code = line.split("#", 1)[0].split(";", 1)[0].strip() + if code.startswith("import") and code[6:7] in (" ", "\t"): + for part in code[6:].split(","): + words = part.split() + if words and _IMPORT_LINE_NAME.fullmatch(words[0]): + names.append((0, words[0])) + elif code.startswith("from") and (found := _FROM_LINE.fullmatch(code)): + dots, module, tail = found.groups() + level, module = len(dots), module or "" + if "(" in tail and ")" not in tail: + # The rest of a parenthesised list, up to the line that closes it. + for more in lines: + more = more.split("#", 1)[0] + tail += " " + more + if ")" in more: + break + if module: + names.append((level, module)) + for part in tail.replace("(", " ").replace(")", " ").replace("\\", " ").split(","): + words = part.split() + if words and _IMPORT_LINE_NAME.fullmatch(words[0]): + names.append((level, f"{module}.{words[0]}".lstrip("."))) + return names + + def _script_imports(content: str) -> list[str]: # Here because agent_type_check imports this module. from .agent_type_check import _SCRIPT_COMMENT_OR_STRING, _script_kept @@ -1406,6 +1458,8 @@ def imported_by_agent( import_limit: int | None = None, lookup_limit: int | None = None, stopped: list[bool] | None = None, + limit_cut: list[bool] | None = None, + search_total: int | None = None, ) -> frozenset[Path]: """The files the agent's folder imports, followed as far as they go. @@ -1423,7 +1477,33 @@ def imported_by_agent( or a link. A reached ``.py`` holding ``import`` that the parser doesn't read in the reading Python runs counts too, whatever the reason: a syntax error, newer syntax, or a template, since its imports can't be known. + + A file the parse total or the parse size keeps from being parsed, and a script the script + budget refuses, aren't left unknown: their import lines are searched instead (``_import_lines``, + ``_script_imports``), and what they name is followed under every other limit. That search has + one total for the walk, ``search_total`` bytes (``_UNREAD_SEARCH_TOTAL`` when not given), and + ``_LARGEST_UNREAD_SEARCHED`` for a file. + + Given ``limit_cut``, its one item says the narrower thing: a limit left code the walk reached + unread, and the line search didn't cover it. That is targets still queued when the loop ended + (the file, byte, import or lookup limit), a file or script that search had no room for, a + code file over the size cap, names cut by the import limit, or the lookups spent with a name + untried. A file the parse total skipped but the search covered, a name dropped for its length + or depth, a link, a file the parser refuses (syntax error) and a codec refusal are not. """ + from .agent_type_check import _LARGEST_UNREAD_SEARCHED, _UNREAD_SEARCH_TOTAL + + search_left = [_UNREAD_SEARCH_TOTAL if search_total is None else search_total] + # Bundling, given no search total, keeps its read cap: only the save's walk searches past it. + searches = search_total is not None + + def search_room(size: int) -> bool: + """Charge ``size`` bytes to the line search's total, if it and the file's cap allow.""" + if size > _LARGEST_UNREAD_SEARCHED or size > search_left[0]: + return False + search_left[0] -= size + return True + root = Path(root).resolve() try: folder = agent_folder(root, subfolder) @@ -1463,6 +1543,8 @@ def imported_by_agent( # Whether the walk left code it reached unfollowed: a limit, a name dropped for its length or # depth, or a file it couldn't read or parse. unread = [members_cut] + # The narrower question: whether a limit, not the code's own shape, left code unread. + limited = [False] def found_at(candidates: Iterable[Path]) -> Path | None: for candidate in candidates: @@ -1470,7 +1552,7 @@ def found_at(candidates: Iterable[Path]) -> Path | None: hit = looked_up[candidate] elif lookups_left[0] <= 0: # A name left untried: the walk didn't reach all it could. - unread[0] = True + unread[0] = limited[0] = True return None else: place = candidate @@ -1488,7 +1570,7 @@ def found_at(candidates: Iterable[Path]) -> Path | None: lookups_left[0] -= 1 hit = None else: - hit = _file_inside(place, root, deepest, lookups_left, unread) + hit = _file_inside(place, root, deepest, lookups_left, unread, limited) if hit is not None or candidate.name == "__init__.py": looked_up[candidate] = hit if hit is not None: @@ -1509,7 +1591,7 @@ def python_modules(path: Path, level: int, name: str) -> list[Path]: for start in starts: if lookups_left[0] <= 0: # Spent: each place left would be refused uncharged. - unread[0] = True + unread[0] = limited[0] = True return [] if len(start.parts) + len(module) > deepest: # Past the depth cut wherever it's looked for: not built at all. @@ -1568,16 +1650,22 @@ def script_module(path: Path, name: str) -> Path | None: if len(path.parts) > deepest: unread[0] = True continue - if not _plain_file(path, unread): - continue + over_cap = [False] + if not _plain_file(path, unread, over_cap): + if not (over_cap[0] and searches): + limited[0] |= over_cap[0] + continue + # A regular code file over the read cap, in a walk that searches: its import lines are + # searched, up to the cap for an unread file, where a parse would refuse it. + largest = _LARGEST_UNREAD_SEARCHED if over_cap[0] else _MAX_FILE_BYTES read += 1 try: with path.open("rb") as handle: - raw = handle.read(_MAX_FILE_BYTES + 1) + raw = handle.read(largest + 1) except OSError: continue - if len(raw) > _MAX_FILE_BYTES: - unread[0] = True + if len(raw) > largest: + unread[0] = limited[0] = True continue # Grew since it was looked at. bytes_read += len(raw) # A .py in a codec that can spell a name in other bytes is read as Python reads it while @@ -1621,14 +1709,31 @@ def script_module(path: Path, name: str) -> Path | None: readings = [content] if read_before is None else [content, read_before] # Each reading is charged as it's parsed: a file parsed twice and charged once let a # crafted pair cost the walk twice its total. - results = [_python_imports(reading, parse_limit, parse_budget) for reading in readings] + budget = parse_budget + results = [] + refused = [] # Whether the parse total or the parse size kept a reading from parsing. + for reading in readings: + size = utf8_size(reading) + refused.append(size > parse_limit or (budget is not None and budget.left < size)) + results.append(_python_imports(reading, parse_limit, budget)) # The decoded reading is the one Python runs: refused, the walk left code unfollowed, # whatever the other reading gives. A refusal of the other alone says nothing. unread[0] |= results[0] is None - unread[0] |= parse_budget is not None and parse_budget.left < 0 + unread[0] |= budget is not None and budget.left < 0 + if any(refused): + # Not parsed for a limit, a syntax error being no limit: its import lines are + # searched instead, which is a cut only when the search has no room. + if search_room(len(raw)): + results = [ + _import_lines(reading) if skipped else result + for reading, result, skipped in zip(readings, results, refused) + ] + else: + limited[0] = True names = list(dict.fromkeys(name for found in results if found for name in found)) # One file can hold more names than the walk may try; the rest aren't tried. unread[0] |= len(names) > import_limit - imports_tried + limited[0] |= len(names) > import_limit - imports_tried names = names[: import_limit - imports_tried] imports_tried += len(names) for level, name in names: @@ -1636,12 +1741,17 @@ def script_module(path: Path, name: str) -> Path | None: hits.extend(python_modules(path, level, name)) except (IndexError, ValueError, OSError): continue # An import that can't be a path in this repository. - elif script_budget is not None and not script_budget.spend(content): - unread[0] = True - continue else: + if script_budget is not None and not script_budget.spend(content): + # Past the script total: the same search the parsed files get, charged to the + # search total, since this is all that bounds it. + unread[0] = True + if not search_room(len(raw)): + limited[0] = True + continue names = list(dict.fromkeys(_script_imports(content))) unread[0] |= len(names) > import_limit - imports_tried + limited[0] |= len(names) > import_limit - imports_tried names = names[: import_limit - imports_tried] imports_tried += len(names) for name in names: @@ -1656,6 +1766,9 @@ def script_module(path: Path, name: str) -> Path | None: pending.append(hit) # A file in a codec decoded first, read without decoding once the total ran out. unread[0] |= escapes.left < 0 + if limit_cut is not None: + # The loop ends with files left only when a limit ended it. + limit_cut[:] = [limited[0] or bool(pending)] if stopped is not None: # The loop ends with files left only when a limit ended it. stopped[:] = [unread[0] or bool(pending)] diff --git a/tests/harness/test_agent_type_refusal.py b/tests/harness/test_agent_type_refusal.py index 470e650af..e62b4a3b2 100644 --- a/tests/harness/test_agent_type_refusal.py +++ b/tests/harness/test_agent_type_refusal.py @@ -3469,10 +3469,18 @@ def test_the_scan_parses_python_only_up_to_its_total(tmp_path, monkeypatch, tota assert browser_tools_found(repo) == expected -def test_the_walk_while_the_conversation_waits_follows_only_small_files(tmp_path): +@pytest.mark.parametrize("search, expected", [(None, ["playwright"]), (1, [])]) +def test_the_walk_while_the_conversation_waits_parses_only_small_files( + tmp_path, monkeypatch, search, expected +): from fi.alk.harness.credentials import imported_by_agent from fi.alk.harness.python_source import LARGEST_PARSED_MODULE + # A file over the parse limit isn't parsed; its import lines are searched instead, so what it + # names is still followed, until the walk's search total has no room for it. + if search is not None: + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", search) + repo = _tree( tmp_path / "repo", { @@ -3482,14 +3490,17 @@ def test_the_walk_while_the_conversation_waits_follows_only_small_files(tmp_path }, ) - assert browser_tools_in_agent(repo, "agents/a") == [] + assert browser_tools_in_agent(repo, "agents/a") == expected # Bundling, with no conversation waiting, still follows it. assert (repo / "common" / "web.py").resolve() in imported_by_agent(repo, "agents/a") -@pytest.mark.parametrize("total, expected", [(36, ["playwright"]), (35, [])]) +@pytest.mark.parametrize( + "total, search, expected", + [(36, None, ["playwright"]), (35, None, ["playwright"]), (35, 1, [])], +) def test_the_walk_while_the_conversation_waits_parses_only_up_to_its_total( - tmp_path, monkeypatch, total, expected + tmp_path, monkeypatch, total, search, expected ): from fi.alk.harness import agent_type_check @@ -3503,6 +3514,9 @@ def test_the_walk_while_the_conversation_waits_parses_only_up_to_its_total( ) # The walk's own total: the browser scan's stays as it is. monkeypatch.setattr(agent_type_check, "_WALKED_PARSED", total) + # Past it a file's import lines are searched, not parsed, unless that has no room either. + if search is not None: + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", search) assert browser_tools_in_agent(repo, "agents/a") == expected @@ -3742,10 +3756,16 @@ def search(self, text, position): @pytest.mark.parametrize( - "budget, followed", [(len(_SHARED_IMPORT), True), (len(_SHARED_IMPORT) - 1, False)] + "budget, search, followed", + [ + (len(_SHARED_IMPORT), None, True), + # Past the script total the imports are matched under the search total instead. + (len(_SHARED_IMPORT) - 1, None, True), + (len(_SHARED_IMPORT) - 1, 1, False), + ], ) def test_the_walk_while_the_conversation_waits_reads_scripts_only_up_to_a_total( - tmp_path, monkeypatch, budget, followed + tmp_path, monkeypatch, budget, search, followed ): from fi.alk.harness.credentials import imported_by_agent @@ -3755,6 +3775,8 @@ def test_the_walk_while_the_conversation_waits_reads_scripts_only_up_to_a_total( {"agents/a/main.js": script, "common/web.js": 'const server = "@playwright/mcp";\n'}, ) monkeypatch.setattr(agent_type_check, "_WALKED_SCRIPTS", budget) + if search is not None: + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", search) assert browser_tools_in_agent(repo, "agents/a") == (["Playwright MCP"] if followed else []) # Bundling, with no conversation waiting, reads them all. @@ -3893,7 +3915,11 @@ def test_the_walk_counts_its_parse_limit_in_bytes(tmp_path): ) shared = (repo / "common" / "web.py").resolve() - assert shared not in imported_by_agent(repo, "agents/a", parse_limit=128 * 1024) + # 128 KB of bytes is over a 128 KB limit once the comment's two-byte characters count; with no + # room to search it, the import isn't followed. + assert shared not in imported_by_agent( + repo, "agents/a", parse_limit=128 * 1024, search_total=0 + ) assert shared in imported_by_agent(repo, "agents/a", parse_limit=256 * 1024) @@ -4434,9 +4460,9 @@ def test_the_walk_charges_a_file_found_for_each_part_of_its_path(tmp_path, monke charged: list[tuple[Path, float]] = [] real = credentials._file_inside - def spy(candidate, root, deepest, left=None, unread=None): + def spy(candidate, root, deepest, left=None, unread=None, too_big=None): before = left[0] - hit = real(candidate, root, deepest, left, unread) + hit = real(candidate, root, deepest, left, unread, too_big) charged.append((candidate, before - left[0])) return hit @@ -5607,7 +5633,7 @@ def test_a_stopped_save_walk_logs_how_many_files_it_reached(tmp_path, caplog, mo ).resolve() monkeypatch.setattr(agent_type_check, "_WALKED_LOOKUPS", 200) with caplog.at_level("WARNING", logger=agent_type_check.__name__): - found = agent_type_check._walked(repo, "agents/a") + found, _ = agent_type_check._walked(repo, "agents/a") assert f"stopped at its limits or left code unread; {len(found)} files reached" in caplog.text assert len(found) >= 1 @@ -6024,7 +6050,7 @@ def test_plain_files_ahead_of_the_agents_imports_dont_use_up_the_parse_total(tmp }, ).resolve() - assert repo / "common" / "web.py" in agent_type_check._walked(repo, "agents/a") + assert repo / "common" / "web.py" in agent_type_check._walked(repo, "agents/a")[0] def test_a_file_the_parse_total_cuts_says_the_walk_stopped(tmp_path): @@ -6222,6 +6248,7 @@ def _save_walk(repo: Path, subfolder: str = "agents/a"): import_limit=agent_type_check._WALKED_IMPORTS, lookup_limit=agent_type_check._WALKED_LOOKUPS, stopped=stopped, + search_total=agent_type_check._WALKED_SEARCHED, ) return found, stopped[0] @@ -6245,9 +6272,6 @@ def _behind(tmp_path: Path, helper: bytes) -> Path: marks=pytest.mark.skipif(sys.version_info >= (3, 13), reason="parses on 3.13"), ), pytest.param(b"from common import web\nNAME = 'a" + "\u0301".encode() * 40 + b"'\n", id="marks"), - pytest.param( - b"# coding: latin-1\nfrom common import web\n# " + b"\xe9" * 70_000 + b"\n", id="oversize-as-utf8" - ), # Any syntax error counts: the parser can't tell newer syntax from Python 2 or a template. pytest.param(b"from common import web\nprint 'py2'\n", id="python-2"), pytest.param(b"{% if web %}\nfrom common import web\n{% endif %}\n", id="template"), @@ -6260,14 +6284,26 @@ def test_a_file_the_walk_cant_parse_says_it_stopped(tmp_path, helper): assert stopped and repo / "common" / "web.py" not in found -def test_a_file_past_the_parse_total_says_the_walk_stopped(tmp_path, monkeypatch): +def test_a_file_over_the_parse_limit_as_utf8_says_stopped_and_is_line_searched(tmp_path): + helper = b"# coding: latin-1\nfrom common import web\n# " + b"\xe9" * 70_000 + b"\n" + repo = _behind(tmp_path, helper) + + found, stopped = _save_walk(repo) + assert stopped and repo / "common" / "web.py" in found + + +@pytest.mark.parametrize("search, followed", [(None, True), (1, False)]) +def test_a_file_past_the_parse_total_says_the_walk_stopped(tmp_path, monkeypatch, search, followed): from fi.alk.harness import agent_type_check repo = _behind(tmp_path, b"from common import web\n" + b"x = 1\n" * 50) monkeypatch.setattr(agent_type_check, "_WALKED_PARSED", 40) + if search is not None: + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", search) + # Not parsed, so the walk says it stopped; its import lines are searched unless that has no room. found, stopped = _save_walk(repo) - assert stopped and repo / "common" / "web.py" not in found + assert stopped and (repo / "common" / "web.py" in found) is followed _ESCAPED_LINE = b"# coding: unicode-escape\n" @@ -6335,7 +6371,7 @@ def test_a_file_over_the_read_limit_says_the_walk_stopped(tmp_path, monkeypatch, real = credentials._plain_file # Small when looked at, past the limit once read. monkeypatch.setattr( - credentials, "_plain_file", lambda path, unread=None: path.name == "main.py" or real(path, unread) + credentials, "_plain_file", lambda path, unread=None, too_big=None: path.name == "main.py" or real(path, unread, too_big) ) found, stopped = _save_walk(repo) diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index f755234cc..f6d6b51ab 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -808,14 +808,68 @@ def test_a_large_honest_repository_gets_a_look(tmp_path, monkeypatch): assert _text(answer) == _look(_CUT_CHAT.format(claimed="chat")) -def test_the_walks_stop_is_no_cut(tmp_path, caplog): - # Over the walk's 512 KB parse total in modules that import, none of them naming a tool. - module = "import os\n" + "x = 1\n" * (100 * 1024 // 6) +_PADDING = "import os\n" + "x = 1\n" * (100 * 1024 // 6) + + +def _padded(root: Path, **more: str) -> Path: + # Six modules that import, over the walk's 512 KB parse total, are read before z_main.py. + return _tree( + root, + { + **{f"agents/a/m_{at}.py": _PADDING for at in range(6)}, + "agents/a/z_main.py": "import common.util\n", + "common/__init__.py": "", + **more, + }, + ) + + +def test_a_file_the_parse_total_skipped_is_line_searched_and_no_cut(tmp_path, caplog): + # Dileep's C2: z_main.py is never parsed, so common.util used to go unfollowed. + repo = _padded(tmp_path / "source", **{"common/util.py": "import playwright\n"}) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + assert any("stopped at its limits" in record.getMessage() for record in caplog.records) + + +def test_a_name_in_a_file_only_the_line_search_reached_is_found_and_no_cut(tmp_path): + # common/deep.py is reached only through util.py's import line, and util.py only through + # z_main.py's: neither was parsed. + repo = _padded( + tmp_path / "source", + **{"common/util.py": "import common.deep\n", "common/deep.py": "import playwright\n"}, + ) + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + + +def test_a_line_search_with_no_room_left_is_a_cut(tmp_path, monkeypatch): + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", 8) + repo = _padded(tmp_path / "source", **{"common/util.py": "import playwright\n"}) + assert browser_scan(repo, "agents/a") == ([], frozenset({"import_walk"})) + answer = _submitter(tmp_path / "out", repo, "agents/a")(_reading("chat")) + assert _text(answer) == _look(_CUT_CHAT.format(claimed="chat")) + assert decide("chat", [], looked_again=False, cut=frozenset({"import_walk"})) == "second_look" + + +def test_a_name_dropped_for_its_depth_alone_is_no_cut(tmp_path, caplog): + deep = ".".join(f"p{at}" for at in range(80)) + repo = _tree(tmp_path / "source", {"agents/a/agent.py": f"import {deep}\nx = 1\n"}) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + assert browser_scan(repo, "agents/a") == ([], frozenset()) + # The walk still logs that it left a name unread. + assert any( + "stopped at its limits" in record.getMessage() for record in caplog.records + ) + answer = _submitter(tmp_path / "out", repo, "agents/a")(_reading("chat")) + assert not answer.get("is_error"), answer + + +def test_a_file_python_refuses_alone_is_no_cut(tmp_path, caplog): repo = _tree( tmp_path / "source", { - **{f"agents/a/m_{at}.py": module for at in range(6)}, - "agents/a/z_main.py": "import common.util\n", + "agents/a/agent.py": "import common.util\n", + "agents/a/broken.py": "import os\ndef (:\n", "common/__init__.py": "", "common/util.py": "x = 1\n", }, @@ -825,10 +879,25 @@ def test_the_walks_stop_is_no_cut(tmp_path, caplog): assert any( "stopped at its limits" in record.getMessage() for record in caplog.records ) - output = tmp_path / "out" - answer = _submitter(output, repo, "agents/a")(_reading("chat")) + answer = _submitter(tmp_path / "out", repo, "agents/a")(_reading("chat")) assert not answer.get("is_error"), answer - assert load(output).modality == "chat" + + +def test_targets_left_in_the_queue_by_the_import_limit_are_a_cut(tmp_path, monkeypatch): + # Two imports are the walk's whole allowance, so the two modules they reach are queued, not + # read: common/m2.py may import anything. + monkeypatch.setattr(agent_type_check, "_WALKED_IMPORTS", 2) + repo = _tree( + tmp_path / "source", + { + "agents/a/agent.py": "import common.m1\nimport common.m2\n", + "common/__init__.py": "", + "common/m1.py": "x = 1\n", + "common/m2.py": "import common.m3\n", + "common/m3.py": "import playwright\n", + }, + ) + assert browser_scan(repo, "agents/a") == ([], frozenset({"import_walk"})) def test_ci_runs_this_file(): From 18e63950ecafa67cf42364811ac5dc9ad2cf6136 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 15:22:15 +0530 Subject: [PATCH 6/8] fix(harness): bound the folder's large-file search, and read an import after ';' or ':' (TH-9512) A folder .py over the 4 MiB search cap is no longer opened, and counts as an import_walk cut; every byte read is charged to the walk. The line search also reads an import after a ';' and after a compound statement's ':'. The parse-refusal search runs only when a search total is given. Signed-off-by: khushalsonawat --- src/fi/alk/harness/credentials.py | 90 +++++++++++++++++------- tests/harness/test_agent_type_refusal.py | 2 + tests/harness/test_browser_scan_cut.py | 43 +++++++++++ 3 files changed, 108 insertions(+), 27 deletions(-) diff --git a/src/fi/alk/harness/credentials.py b/src/fi/alk/harness/credentials.py index c8fc5d927..eefed5d21 100644 --- a/src/fi/alk/harness/credentials.py +++ b/src/fi/alk/harness/credentials.py @@ -1353,41 +1353,67 @@ def _python_imports( _FROM_LINE = re.compile(r"from[ \t]+(\.*)[ \t]*([A-Za-z_][\w.]*)?[ \t]+import\b(.*)", re.DOTALL) +_COMPOUND_START = re.compile( + r"(?:if|elif|else|for|while|try|except|finally|with|async|def|class|match|case)\b" +) +_MOST_COLONS_TRIED = 8 + + +def _statements(code: str) -> list[str]: + """The pieces of one source line (its comment already cut) a statement can start at: each + ``;`` segment, and for a segment that opens a compound statement (``if True: import x``) the + text after each of its first few ``:``. A piece that is no import is skipped by the caller. + A line search, so a ``;`` or ``:`` inside a string splits it too, which only adds a candidate.""" + pieces: list[str] = [] + for segment in code.split(";"): + segment = segment.strip() + pieces.append(segment) + if _COMPOUND_START.match(segment): + at = -1 + for _ in range(_MOST_COLONS_TRIED): + at = segment.find(":", at + 1) + if at < 0: + break + pieces.append(segment[at + 1 :].strip()) + return pieces + + def _import_lines(content: str) -> list[tuple[int, str]]: """The names a line search finds ``content`` importing, each with its level, in the shape ``_python_imports`` gives: ``import a.b, c`` and ``from .x import y`` / ``from a.b import c`` - at the start of a line (indented or not), whatever follows a ``#`` or a ``;`` ignored. A + at the start of a line (indented or not), a ``;`` segment, or the text after a compound + statement's ``:``, whatever follows a ``#`` ignored. A parenthesised list is read across its lines. No parse, so no budget: the caller charges the bytes to the walk's search total. It misses an import built at run time (``importlib.import_module(name)``, ``__import__``), one - inside a multi-line string or after a ``;``, and a name only a decoded codec spells. + inside a multi-line string, and a name only a decoded codec spells. """ names: list[tuple[int, str]] = [] lines = iter(content.split("\n")) for line in lines: - code = line.split("#", 1)[0].split(";", 1)[0].strip() - if code.startswith("import") and code[6:7] in (" ", "\t"): - for part in code[6:].split(","): - words = part.split() - if words and _IMPORT_LINE_NAME.fullmatch(words[0]): - names.append((0, words[0])) - elif code.startswith("from") and (found := _FROM_LINE.fullmatch(code)): - dots, module, tail = found.groups() - level, module = len(dots), module or "" - if "(" in tail and ")" not in tail: - # The rest of a parenthesised list, up to the line that closes it. - for more in lines: - more = more.split("#", 1)[0] - tail += " " + more - if ")" in more: - break - if module: - names.append((level, module)) - for part in tail.replace("(", " ").replace(")", " ").replace("\\", " ").split(","): - words = part.split() - if words and _IMPORT_LINE_NAME.fullmatch(words[0]): - names.append((level, f"{module}.{words[0]}".lstrip("."))) + for code in _statements(line.split("#", 1)[0]): + if code.startswith("import") and code[6:7] in (" ", "\t"): + for part in code[6:].split(","): + words = part.split() + if words and _IMPORT_LINE_NAME.fullmatch(words[0]): + names.append((0, words[0])) + elif code.startswith("from") and (found := _FROM_LINE.fullmatch(code)): + dots, module, tail = found.groups() + level, module = len(dots), module or "" + if "(" in tail and ")" not in tail: + # The rest of a parenthesised list, up to the line that closes it. + for more in lines: + more = more.split("#", 1)[0] + tail += " " + more + if ")" in more: + break + if module: + names.append((level, module)) + for part in tail.replace("(", " ").replace(")", " ").replace("\\", " ").split(","): + words = part.split() + if words and _IMPORT_LINE_NAME.fullmatch(words[0]): + names.append((level, f"{module}.{words[0]}".lstrip("."))) return names @@ -1658,16 +1684,25 @@ def script_module(path: Path, name: str) -> Path | None: # A regular code file over the read cap, in a walk that searches: its import lines are # searched, up to the cap for an unread file, where a parse would refuse it. largest = _LARGEST_UNREAD_SEARCHED if over_cap[0] else _MAX_FILE_BYTES + if over_cap[0]: + # Over the search's cap for a file: not opened, a limit cut as an unread file is. + try: + too_big = path.lstat().st_size > largest + except (OSError, ValueError): + continue + if too_big: + unread[0] = limited[0] = True + continue read += 1 try: with path.open("rb") as handle: raw = handle.read(largest + 1) except OSError: continue + bytes_read += len(raw) if len(raw) > largest: unread[0] = limited[0] = True continue # Grew since it was looked at. - bytes_read += len(raw) # A .py in a codec that can spell a name in other bytes is read as Python reads it while # the walk's total lasts: decoded after its line ends, so hz's `~` before a CRLF joins. taken, decoded, encoding = ( @@ -1722,8 +1757,9 @@ def script_module(path: Path, name: str) -> Path | None: unread[0] |= budget is not None and budget.left < 0 if any(refused): # Not parsed for a limit, a syntax error being no limit: its import lines are - # searched instead, which is a cut only when the search has no room. - if search_room(len(raw)): + # searched instead (in a walk that searches), which is a cut only when the + # search has no room. + if searches and search_room(len(raw)): results = [ _import_lines(reading) if skipped else result for reading, result, skipped in zip(readings, results, refused) diff --git a/tests/harness/test_agent_type_refusal.py b/tests/harness/test_agent_type_refusal.py index e62b4a3b2..7ef893642 100644 --- a/tests/harness/test_agent_type_refusal.py +++ b/tests/harness/test_agent_type_refusal.py @@ -3921,6 +3921,8 @@ def test_the_walk_counts_its_parse_limit_in_bytes(tmp_path): repo, "agents/a", parse_limit=128 * 1024, search_total=0 ) assert shared in imported_by_agent(repo, "agents/a", parse_limit=256 * 1024) + # The refusal search is the save's alone: given no search total, the walk's answer is as it was. + assert shared not in imported_by_agent(repo, "agents/a", parse_limit=128 * 1024) def _nested_packages(repo: Path, depth: int) -> Path: diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index f6d6b51ab..f08119d49 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -900,6 +900,49 @@ def test_targets_left_in_the_queue_by_the_import_limit_are_a_cut(tmp_path, monke assert browser_scan(repo, "agents/a") == ([], frozenset({"import_walk"})) +@pytest.mark.parametrize( + "line", + ["x = 1; import common.util", "if True: import common.util", "x = 1;import common.util"], +) +def test_an_import_after_a_semicolon_or_a_colon_is_line_searched_and_no_cut(tmp_path, line): + # Dileep's padding keeps z_main.py from being parsed; its import isn't at the line's start. + repo = _padded( + tmp_path / "source", + **{"agents/a/z_main.py": f"{line}\n", "common/util.py": "import playwright\n"}, + ) + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + + +def test_a_file_over_the_search_cap_is_not_opened_and_is_a_cut(tmp_path, monkeypatch): + # Ten sparse 5 MiB files in the agent's folder: each is over the 4 MiB a file is searched at, + # so none is opened (the bytes used to be read, then dropped uncharged). + repo = _tree(tmp_path / "source", {"agents/a/agent.py": "x = 1\n"}) + for at in range(10): + with (repo / f"agents/a/big_{at}.py").open("wb") as handle: + handle.truncate(5 * 1024 * 1024) + read = [0] + real_open = Path.open + + def counting_open(self, mode="r", *args, **kwargs): + handle = real_open(self, mode, *args, **kwargs) + if not self.name.startswith("big_"): + return handle + real_read = handle.read + + def read_counted(size=-1): + data = real_read(size) + read[0] += len(data) + return data + + handle.read = read_counted + return handle + + monkeypatch.setattr(Path, "open", counting_open) + names, cut = browser_scan(repo, "agents/a") + assert names == [] and "import_walk" in cut + assert read[0] <= agent_type_check._LARGEST_UNREAD_SEARCHED + + def test_ci_runs_this_file(): workflows = ( Path(__file__).resolve().parents[2] / ".github" / "workflows" / "sdk-smoke.yml" From 0e2570afb393d1dc67c1eb7bcc07572d1e4f1d16 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 16:07:42 +0530 Subject: [PATCH 7/8] fix(harness): cut a reading whose import sits past the colons the line search tries (TH-9512) The line search tries a compound statement's first 8 colons. A line with more before its import wasn't followed and wasn't cut, so the import went unseen with no second look. It is now a limit cut, like the walk's other caps; a long line with no import after its colons stays no cut. Signed-off-by: khushalsonawat --- src/fi/alk/harness/credentials.py | 23 +++++++++++++++++------ tests/harness/test_browser_scan_cut.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/fi/alk/harness/credentials.py b/src/fi/alk/harness/credentials.py index eefed5d21..fb8ce7a30 100644 --- a/src/fi/alk/harness/credentials.py +++ b/src/fi/alk/harness/credentials.py @@ -1359,11 +1359,13 @@ def _python_imports( _MOST_COLONS_TRIED = 8 -def _statements(code: str) -> list[str]: +def _statements(code: str, capped: list[bool] | None = None) -> list[str]: """The pieces of one source line (its comment already cut) a statement can start at: each ``;`` segment, and for a segment that opens a compound statement (``if True: import x``) the text after each of its first few ``:``. A piece that is no import is skipped by the caller. - A line search, so a ``;`` or ``:`` inside a string splits it too, which only adds a candidate.""" + A line search, so a ``;`` or ``:`` inside a string splits it too, which only adds a candidate. + Given ``capped``, its one item is set when a segment has more colons than are tried and an + ``import`` after the last one tried: that import isn't searched, so the caller cuts.""" pieces: list[str] = [] for segment in code.split(";"): segment = segment.strip() @@ -1375,10 +1377,14 @@ def _statements(code: str) -> list[str]: if at < 0: break pieces.append(segment[at + 1 :].strip()) + else: + rest = segment[at + 1 :] + if capped is not None and ":" in rest and "import" in rest: + capped[0] = True return pieces -def _import_lines(content: str) -> list[tuple[int, str]]: +def _import_lines(content: str, capped: list[bool] | None = None) -> list[tuple[int, str]]: """The names a line search finds ``content`` importing, each with its level, in the shape ``_python_imports`` gives: ``import a.b, c`` and ``from .x import y`` / ``from a.b import c`` at the start of a line (indented or not), a ``;`` segment, or the text after a compound @@ -1387,12 +1393,13 @@ def _import_lines(content: str) -> list[tuple[int, str]]: bytes to the walk's search total. It misses an import built at run time (``importlib.import_module(name)``, ``__import__``), one - inside a multi-line string, and a name only a decoded codec spells. + inside a multi-line string, and a name only a decoded codec spells. Given ``capped``, its one + item is set when a line had an import past the colons tried (``_statements``). """ names: list[tuple[int, str]] = [] lines = iter(content.split("\n")) for line in lines: - for code in _statements(line.split("#", 1)[0]): + for code in _statements(line.split("#", 1)[0], capped): if code.startswith("import") and code[6:7] in (" ", "\t"): for part in code[6:].split(","): words = part.split() @@ -1760,10 +1767,14 @@ def script_module(path: Path, name: str) -> Path | None: # searched instead (in a walk that searches), which is a cut only when the # search has no room. if searches and search_room(len(raw)): + capped = [False] results = [ - _import_lines(reading) if skipped else result + _import_lines(reading, capped) if skipped else result for reading, result, skipped in zip(readings, results, refused) ] + # An import past the colons tried isn't followed: a cut, never silent. + limited[0] |= capped[0] + unread[0] |= capped[0] else: limited[0] = True names = list(dict.fromkeys(name for found in results if found for name in found)) diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index f08119d49..3810aac1f 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -913,6 +913,23 @@ def test_an_import_after_a_semicolon_or_a_colon_is_line_searched_and_no_cut(tmp_ assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) +def test_an_import_past_the_colon_cap_is_a_cut(tmp_path): + # The line search tries a compound statement's first 8 colons; one with more before its + # import isn't followed, so the walk left code unread and the reading is cut, not silent. + line = "if a[1:2:3:4:5:6:7:8:9]: import common.util" + repo = _padded( + tmp_path / "source", + **{"agents/a/z_main.py": f"{line}\n", "common/util.py": "import playwright\n"}, + ) + assert browser_scan(repo, "agents/a") == ([], frozenset({"import_walk"})) + + +def test_a_line_past_the_colon_cap_with_no_import_after_is_no_cut(tmp_path): + line = "if a[1:2:3:4:5:6:7:8:9]: x = 1" + repo = _padded(tmp_path / "source", **{"agents/a/z_main.py": f"{line}\nimport os\n"}) + assert browser_scan(repo, "agents/a") == ([], frozenset()) + + def test_a_file_over_the_search_cap_is_not_opened_and_is_a_cut(tmp_path, monkeypatch): # Ten sparse 5 MiB files in the agent's folder: each is over the 4 MiB a file is searched at, # so none is opened (the bytes used to be read, then dropped uncharged). From 7084b31e34688578cde2b55f92d43eda4eadad35 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Thu, 12 Nov 2026 17:07:18 +0530 Subject: [PATCH 8/8] fix(harness): a line search with no backtracking, the legal import forms, and cut where it can't be sure (TH-9512) The line search moves to import_lines.py: strings and comments are taken out in one pass, then the import lines are read by splitting on whitespace, with no regular expression that can backtrack. A line continued by a backslash is joined, and from.x, from .import x, a # inside a string and a docstring holding an open paren are read as Python reads them. A string, list or colon run it cannot be sure of sets capped, so the reading is cut. The walk also reaches a code file over the read cap, up to the search cap, and searches it. Signed-off-by: khushalsonawat --- .github/workflows/sdk-smoke.yml | 1 + src/fi/alk/harness/agent_type_check.py | 112 +++++------ src/fi/alk/harness/credentials.py | 138 ++++--------- src/fi/alk/harness/import_lines.py | 246 +++++++++++++++++++++++ tests/harness/test_agent_type_refusal.py | 19 +- tests/harness/test_browser_scan_cut.py | 201 +++++++++++++++++- tests/harness/test_import_lines.py | 238 ++++++++++++++++++++++ 7 files changed, 795 insertions(+), 160 deletions(-) create mode 100644 src/fi/alk/harness/import_lines.py create mode 100644 tests/harness/test_import_lines.py diff --git a/.github/workflows/sdk-smoke.yml b/.github/workflows/sdk-smoke.yml index 814a3aabc..e09ee7e51 100644 --- a/.github/workflows/sdk-smoke.yml +++ b/.github/workflows/sdk-smoke.yml @@ -101,6 +101,7 @@ jobs: tests/harness/test_agent_reading.py tests/harness/test_scan_walk.py tests/harness/test_browser_scan_cut.py + tests/harness/test_import_lines.py tests/harness/test_scan_folders.py tests/harness/test_scan_document.py tests/harness/test_scan_ai.py diff --git a/src/fi/alk/harness/agent_type_check.py b/src/fi/alk/harness/agent_type_check.py index ac23723b3..1b29b21a1 100644 --- a/src/fi/alk/harness/agent_type_check.py +++ b/src/fi/alk/harness/agent_type_check.py @@ -23,6 +23,12 @@ from .credentials import imported_by_agent from .failure_file import FAILURE_FILE +from .import_lines import ( + LARGEST_SEARCHED, + SCRIPT_COMMENT_OR_STRING as _SCRIPT_COMMENT_OR_STRING, + SEARCH_TOTAL, + script_kept as _script_kept, +) from .instrumentation_evidence import _SKIPPED_DIRECTORIES, _imports from .job import FailureDomain, HarnessFailure, HarnessStage from .python_source import ( @@ -135,6 +141,11 @@ ) for name in BROWSER_IMPORTS } +# The same, as bytes, for the unread files whose bytes are searched (see _may_name_a_tool). +_WHOLE_NAMES_BYTES = [ + (name.encode(), re.compile(pattern.pattern.encode(), re.ASCII)) + for name, pattern in _WHOLE_NAMES.items() +] # A Python file that names none of these holds no browser import or tool marker, so it isn't parsed. _PYTHON_NAMES = ( *(name.encode() for name in sorted(BROWSER_IMPORTS)), @@ -150,13 +161,13 @@ # A file too big to read is searched as bytes for the names that would make it matter (see # _may_name_a_tool), up to this size: a generated or data module of a few MB is cleared. Larger # ones aren't read at all. -_LARGEST_UNREAD_SEARCHED = 4 * 1024 * 1024 +_LARGEST_UNREAD_SEARCHED = LARGEST_SEARCHED # And what one scan searches of them in all. Every name is looked for in each file: about 18 ms a # MiB on 3.12.3, crafted near misses included, so this much is about 0.15 s. A coding line naming a # codec costs more: the lookup normalises the name in Python, so two crafted 4 MiB names took # 0.31 s, and encodings._cache keeps each name it misses, 8 M characters for those two, for as long # as the process runs. read_module looks up the names of files it reads the same way. -_UNREAD_SEARCH_TOTAL = 8 * 1024 * 1024 +_UNREAD_SEARCH_TOTAL = SEARCH_TOTAL # Past this a repository holds the conversation up for longer than the rest of the scan is worth. _LARGEST_SCAN = 128 * 1024 * 1024 # Scripts are read by regular expressions that call back for every string or comment: 128 MB of @@ -195,31 +206,6 @@ # and this is about 0.5 s of them. A whole 411-package environment unpacked in one folder came # to 6.4 MiB. _RECORDS_AT_AND_ABOVE = 16 * 1024 * 1024 -# Strings are matched first and kept, so "https://..." or "src/**/*.ts" isn't read as a comment. -# An unclosed one ends at its line or the file: a start that could fail makes a customer's odd file -# cost the square of its size, which holds up the build's conversation until it's done. -# A regex literal is kept the same way, so the "/*" in /\/*$/ doesn't hide the rest of the file. A -# slash where a value is expected starts one: after punctuation or at a line's start, never after a -# name or a number, where it divides. Its closing slash must be followed as a regex's is, by flags -# and then no name, quote, "/" or "*": a division that starts a line, or a "/" in JSX text, then -# reads as division, not as a literal that eats a string's opening quote. -# Such a start can fail, so a class holds no "[" and the rest no "]", and a literal that meets one -# is kept to its line's end: a failed literal then can't run over the classes of the slashes after -# it, which would make the line cost its square, and the quotes and backticks it holds can't open a -# string or a template that swaps code and strings for the rest of the file. The "]" also stops a -# literal that runs on through escaped line ends, which would cost the square of their number. -# Kept to its line's end, such a literal can take in a template's opening backtick later on that -# line too; a "/*" inside that template then reads as a comment. That trade is the price of a line -# that can't run away. -_SCRIPT_COMMENT_OR_STRING = re.compile( - r"""("(?:\\.|[^"\\\n])*(?:"|(?=\n)|\\?\Z)|'(?:\\.|[^'\\\n])*(?:'|(?=\n)|\\?\Z)|`(?:\\.|[^`\\])*(?:`|\\?\Z)""" - r"""|(?])[ \t]*/(?![/*])(?:\\.|\[(?:\\.|[^\]\[\\\n])*(?:\]|(?=\n)|\\?\Z)|[^/\\\n\[\]])*""" - r"""(?:/(?=[dgimsuvy]*(?![\w$/*"'`]))|(?=\n)|\\?\Z|(?:\[(?=(?:\\.|[^\]\[\\\n])*\[)|\])[^\n]*))""" - r"""|//[^\n]*|/\*.*?(?:\*/|\Z)""", - re.DOTALL, -) - - # The whole name, test data, or a test word and a helper word: "integration_tests" and # "test_utils" hold tests, while "e2e_agent" and "testing_agent" hold agents, and test automation # is a main kind of browser agent. @@ -493,14 +479,6 @@ def _python_code(tokens: list[tokenize.TokenInfo]) -> str: return " ".join(token.string for token in tokens if token.type != tokenize.COMMENT) -def _script_kept(hit: re.Match[str]) -> str: - kept = hit.group(1) or "" - # A lone apostrophe, as in JSX text, isn't a string, so it can't protect a comment after it. - if kept.startswith("'") and not kept.endswith("'"): - return kept.split("//", 1)[0] - return kept - - def _python_markers(text: str, tokens: TokenBudget, path: Path) -> set[str] | None: """The tool markers in ``text``'s code, or None when the scan's tokenizing ran out first.""" if not _MARKER_START.search(text): @@ -551,11 +529,11 @@ def _may_name_a_tool(path: Path, size: int, left: list[int]) -> bool: """Whether a file too big to read, of ``size`` bytes, may name a tool, so leaving it unread cuts the scan. - Its bytes are searched for the names a Python file must hold to be read at all - (``_PYTHON_NAMES``, as ``read_module`` searches them), "omputer" aside: that is a word in - prose, so it counts only where a ``_MARKER_START`` match starts. Every such match holds - "omputer" or "playwright", so this covers a script's tool markers too. It's never decoded or - parsed. A Python file whose coding line names a codec not proven safe to search as bytes can + Its bytes are searched, never decoded or parsed. A Python file may name a tool by a whole + package name (``_WHOLE_NAMES``: "steelblue" is no "steel"), by the toolkit's name, or by a + tool marker, which counts only where a ``_MARKER_START`` match starts: "omputer" alone is a + word in prose. A script is searched for markers only, since a read script contributes + nothing else (``_markers_in``): a package it imports is declared in its ``package.json``. A Python file whose coding line names a codec not proven safe to search as bytes can spell a name its bytes don't show, so it may; so may one whose coding line Python refuses, since the codec can be registered at run time before the file is imported. A file that can't be cleared may: one over ``_LARGEST_UNREAD_SEARCHED``, over what is ``left`` of the scan's @@ -582,9 +560,12 @@ def _may_name_a_tool(path: Path, size: int, left: list[int]) -> bool: coding = _coding(raw) if coding is None or coding[1].name not in _RAW_SAFE_CODECS: return True - if any(name in raw for name in _PYTHON_NAMES if name != b"omputer"): + if path.suffix == ".py" and ( + _TOOLKIT.encode() in raw + or any(name in raw and whole.search(raw) for name, whole in _WHOLE_NAMES_BYTES) + ): return True - return b"omputer" in raw and _MARKER_START_BYTES.search(raw) is not None + return _MARKER_START_BYTES.search(raw) is not None def browser_tools_found(source_root: Path | None) -> list[str]: @@ -667,20 +648,33 @@ def _scan( # The record the budget left unread may be the one naming a package unpacked at the top, # so no folder there shows the agent wrote it: every import counts as the agent's. local = set() if left[0] < 0 else _modules_at_top(root, at_top) - return _tools_in(files, local) + names, cut = _tools_in(files, local) + _log_cut(cut) + return names, frozenset(cut) + + +def _log_cut(cut: Counter[str]) -> None: + """One line for a scan's cuts, once the scan has all of them.""" + if cut: + # Names and counts only: a path can name the customer's business. + logger.warning( + "agent type scan cut by %s", + ", ".join(f"{name} {count}" for name, count in sorted(cut.items())), + ) def _tools_in( paths: Iterable[Path], local: set[str], first: Path | None = None -) -> tuple[tuple[str, ...], frozenset[str]]: +) -> tuple[tuple[str, ...], Counter[str]]: """The browser tools ``paths`` import or name, the files in ``first`` read before the rest: the totals below cut the files read last, and the agent's own mustn't be those. - And the limits that cut the scan, by name: each left code unread or unparsed, so finding - nothing then doesn't say there's nothing, and the reading gets a second look. A file that - names no tool, or that Python can't decode, isn't one: it can't import what Python runs. A - file too big to read is searched as bytes to say whether it names one (``_may_name_a_tool``), - so a large data module that doesn't isn't one either. + And the limits that cut the scan, by name and count: each left code unread or unparsed, so + finding nothing then doesn't say there's nothing, and the reading gets a second look. A file + that names no tool, or that Python can't decode, isn't one: it can't import what Python + runs. A file too big to read is searched as bytes to say whether it names one + (``_may_name_a_tool``), so a large data module that doesn't isn't one either. The caller + logs the cuts (``_log_cut``), once it has added its own. """ found: set[str] = set() imported: set[str] = set() @@ -768,7 +762,7 @@ def _tools_in( # A skipped file may be the agent's main one, and the reading then finds nothing. No # trace at any level, since a trace can quote a line of the customer's code; the type # only at WARNING, once a scan, since a path can name their business. The count is in - # the cut line below. + # the cut line. cut["skipped"] += 1 if cut["skipped"] == 1: logger.warning("agent type scan skipped a file: %s", type(error).__name__) @@ -777,12 +771,6 @@ def _tools_in( if escapes.left < 0: # A file past the total was searched as bytes, which can miss a name its codec spells. cut["decode_total"] = 1 - if cut: - # Names and counts only: a path can name the customer's business. - logger.warning( - "agent type scan cut by %s", - ", ".join(f"{name} {count}" for name, count in sorted(cut.items())), - ) for name in imported: top = name.split(".")[0] if top in local: @@ -791,7 +779,7 @@ def _tools_in( found.add(top) elif name.endswith(f".{_TOOLKIT}"): found.add(_TOOLKIT) - return tuple(sorted({name.replace("_", "-") for name in found})), frozenset(cut) + return tuple(sorted({name.replace("_", "-") for name in found})), cut def _files_in(folder: Path) -> list[Path]: @@ -831,6 +819,7 @@ def _walked(root: Path, subfolder: str) -> tuple[frozenset[Path], bool]: stopped=stopped, limit_cut=limit_cut, search_total=_WALKED_SEARCHED, + search_file_limit=_LARGEST_UNREAD_SEARCHED, ) if stopped and stopped[0]: logger.warning( @@ -948,9 +937,12 @@ def unpacked(path: Path) -> bool: names, cut = _tools_in( dict.fromkeys((*_agent_files(folder), *above, *shared, *manifests)), local, folder ) - # A budget or limit left code the walk reached unread, so what it would have led to is - # unknown (TH-9512, Dileep's C2). _tools_in has already logged its own cuts. - return names, (cut | {"import_walk"} if walk_cut else cut) + if walk_cut: + # A budget or limit left code the walk reached unread, so what it would have led to is + # unknown. + cut["import_walk"] += 1 + _log_cut(cut) + return names, frozenset(cut) def modality_text(value: Any) -> str: diff --git a/src/fi/alk/harness/credentials.py b/src/fi/alk/harness/credentials.py index fb8ce7a30..5a6bd82c6 100644 --- a/src/fi/alk/harness/credentials.py +++ b/src/fi/alk/harness/credentials.py @@ -26,6 +26,13 @@ from fi.simulate.runtime.spec import SecretRef +from .import_lines import ( + LARGEST_SEARCHED, + SEARCH_TOTAL, + SCRIPT_COMMENT_OR_STRING, + import_lines, + script_kept, +) from .python_source import EscapeBudget, ParseBudget, decoded_first, parsed, utf8_size from .secrets import process_secret_environment_names from .source_folder import SubfolderNotFound, agent_folder, agent_scope, in_agent_scope, is_within @@ -678,20 +685,26 @@ def _file_inside( left: list[float] | None = None, unread: list[bool] | None = None, too_big: list[bool] | None = None, + largest: int | None = None, ) -> Path | None: """``candidate`` resolved, if it's a plain file in ``root`` at most ``deepest`` parts long. ``left`` is the lookups on disk still allowed: one for the file, then one for each part of its path, which resolving it looks up in turn. Given ``unread``, its one item is set when a candidate is refused for its depth, or as a code file too big to read or a link to one: code - may be there that the walk doesn't follow. + may be there that the walk doesn't follow. ``largest`` is the size of file it returns, the + read cap when not given: a walk that line-searches code over the read cap gives its own. """ if len(candidate.parts) > deepest: _mark(unread) return None if left is not None: left[0] -= 1 - if not _plain_file(candidate, unread, too_big): + if not ( + _plain_file(candidate, unread, too_big) + if largest is None + else _plain_file(candidate, unread, too_big, largest) + ): return None if left is not None: left[0] -= len(candidate.parts) @@ -1299,7 +1312,10 @@ def _read_in_place(real: Path, folder: Path) -> bool: def _plain_file( - path: Path, unread: list[bool] | None = None, too_big: list[bool] | None = None + path: Path, + unread: list[bool] | None = None, + too_big: list[bool] | None = None, + largest: int = _MAX_FILE_BYTES, ) -> bool: """A regular file small enough to read: a committed link can point at anything, so it isn't followed. Given ``unread``, its one item is set when a code file is refused for its size or @@ -1310,7 +1326,7 @@ def _plain_file( details = path.lstat() except (OSError, ValueError): return False - if stat.S_ISREG(details.st_mode) and details.st_size <= _MAX_FILE_BYTES: + if stat.S_ISREG(details.st_mode) and details.st_size <= largest: return True if ( too_big is not None @@ -1349,86 +1365,8 @@ def _python_imports( return [(level, name) for level, name in names if name] -_IMPORT_LINE_NAME = re.compile(r"[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)*") -_FROM_LINE = re.compile(r"from[ \t]+(\.*)[ \t]*([A-Za-z_][\w.]*)?[ \t]+import\b(.*)", re.DOTALL) - - -_COMPOUND_START = re.compile( - r"(?:if|elif|else|for|while|try|except|finally|with|async|def|class|match|case)\b" -) -_MOST_COLONS_TRIED = 8 - - -def _statements(code: str, capped: list[bool] | None = None) -> list[str]: - """The pieces of one source line (its comment already cut) a statement can start at: each - ``;`` segment, and for a segment that opens a compound statement (``if True: import x``) the - text after each of its first few ``:``. A piece that is no import is skipped by the caller. - A line search, so a ``;`` or ``:`` inside a string splits it too, which only adds a candidate. - Given ``capped``, its one item is set when a segment has more colons than are tried and an - ``import`` after the last one tried: that import isn't searched, so the caller cuts.""" - pieces: list[str] = [] - for segment in code.split(";"): - segment = segment.strip() - pieces.append(segment) - if _COMPOUND_START.match(segment): - at = -1 - for _ in range(_MOST_COLONS_TRIED): - at = segment.find(":", at + 1) - if at < 0: - break - pieces.append(segment[at + 1 :].strip()) - else: - rest = segment[at + 1 :] - if capped is not None and ":" in rest and "import" in rest: - capped[0] = True - return pieces - - -def _import_lines(content: str, capped: list[bool] | None = None) -> list[tuple[int, str]]: - """The names a line search finds ``content`` importing, each with its level, in the shape - ``_python_imports`` gives: ``import a.b, c`` and ``from .x import y`` / ``from a.b import c`` - at the start of a line (indented or not), a ``;`` segment, or the text after a compound - statement's ``:``, whatever follows a ``#`` ignored. A - parenthesised list is read across its lines. No parse, so no budget: the caller charges the - bytes to the walk's search total. - - It misses an import built at run time (``importlib.import_module(name)``, ``__import__``), one - inside a multi-line string, and a name only a decoded codec spells. Given ``capped``, its one - item is set when a line had an import past the colons tried (``_statements``). - """ - names: list[tuple[int, str]] = [] - lines = iter(content.split("\n")) - for line in lines: - for code in _statements(line.split("#", 1)[0], capped): - if code.startswith("import") and code[6:7] in (" ", "\t"): - for part in code[6:].split(","): - words = part.split() - if words and _IMPORT_LINE_NAME.fullmatch(words[0]): - names.append((0, words[0])) - elif code.startswith("from") and (found := _FROM_LINE.fullmatch(code)): - dots, module, tail = found.groups() - level, module = len(dots), module or "" - if "(" in tail and ")" not in tail: - # The rest of a parenthesised list, up to the line that closes it. - for more in lines: - more = more.split("#", 1)[0] - tail += " " + more - if ")" in more: - break - if module: - names.append((level, module)) - for part in tail.replace("(", " ").replace(")", " ").replace("\\", " ").split(","): - words = part.split() - if words and _IMPORT_LINE_NAME.fullmatch(words[0]): - names.append((level, f"{module}.{words[0]}".lstrip("."))) - return names - - def _script_imports(content: str) -> list[str]: - # Here because agent_type_check imports this module. - from .agent_type_check import _SCRIPT_COMMENT_OR_STRING, _script_kept - - code = _without_type_imports(_SCRIPT_COMMENT_OR_STRING.sub(_script_kept, content)) + code = _without_type_imports(SCRIPT_COMMENT_OR_STRING.sub(script_kept, content)) return [hit.group(1) for hit in _SCRIPT_IMPORT_OR_STRING.finditer(code) if hit.group(1)] @@ -1493,6 +1431,7 @@ def imported_by_agent( stopped: list[bool] | None = None, limit_cut: list[bool] | None = None, search_total: int | None = None, + search_file_limit: int | None = None, ) -> frozenset[Path]: """The files the agent's folder imports, followed as far as they go. @@ -1512,10 +1451,12 @@ def imported_by_agent( its imports can't be known. A file the parse total or the parse size keeps from being parsed, and a script the script - budget refuses, aren't left unknown: their import lines are searched instead (``_import_lines``, + budget refuses, aren't left unknown: their import lines are searched instead (``import_lines``, ``_script_imports``), and what they name is followed under every other limit. That search has - one total for the walk, ``search_total`` bytes (``_UNREAD_SEARCH_TOTAL`` when not given), and - ``_LARGEST_UNREAD_SEARCHED`` for a file. + one total for the walk, ``search_total`` bytes, and ``search_file_limit`` for a file (the + caller's caps; ``SEARCH_TOTAL`` and ``LARGEST_SEARCHED`` when not given). A walk that + searches also reaches a regular code file over the read cap, up to that file limit, and + searches it; a larger one is a cut. Given ``limit_cut``, its one item says the narrower thing: a limit left code the walk reached unread, and the line search didn't cover it. That is targets still queued when the loop ended @@ -1524,15 +1465,14 @@ def imported_by_agent( untried. A file the parse total skipped but the search covered, a name dropped for its length or depth, a link, a file the parser refuses (syntax error) and a codec refusal are not. """ - from .agent_type_check import _LARGEST_UNREAD_SEARCHED, _UNREAD_SEARCH_TOTAL - - search_left = [_UNREAD_SEARCH_TOTAL if search_total is None else search_total] + search_left = [SEARCH_TOTAL if search_total is None else search_total] + search_largest = LARGEST_SEARCHED if search_file_limit is None else search_file_limit # Bundling, given no search total, keeps its read cap: only the save's walk searches past it. searches = search_total is not None def search_room(size: int) -> bool: """Charge ``size`` bytes to the line search's total, if it and the file's cap allow.""" - if size > _LARGEST_UNREAD_SEARCHED or size > search_left[0]: + if size > search_largest or size > search_left[0]: return False search_left[0] -= size return True @@ -1603,7 +1543,15 @@ def found_at(candidates: Iterable[Path]) -> Path | None: lookups_left[0] -= 1 hit = None else: - hit = _file_inside(place, root, deepest, lookups_left, unread, limited) + hit = _file_inside( + place, + root, + deepest, + lookups_left, + unread, + limited, + *((search_largest,) if searches else ()), + ) if hit is not None or candidate.name == "__init__.py": looked_up[candidate] = hit if hit is not None: @@ -1684,13 +1632,15 @@ def script_module(path: Path, name: str) -> Path | None: unread[0] = True continue over_cap = [False] - if not _plain_file(path, unread, over_cap): + link_or_big = [False] + if not _plain_file(path, link_or_big, over_cap): if not (over_cap[0] and searches): + unread[0] |= link_or_big[0] limited[0] |= over_cap[0] continue # A regular code file over the read cap, in a walk that searches: its import lines are # searched, up to the cap for an unread file, where a parse would refuse it. - largest = _LARGEST_UNREAD_SEARCHED if over_cap[0] else _MAX_FILE_BYTES + largest = search_largest if over_cap[0] else _MAX_FILE_BYTES if over_cap[0]: # Over the search's cap for a file: not opened, a limit cut as an unread file is. try: @@ -1769,7 +1719,7 @@ def script_module(path: Path, name: str) -> Path | None: if searches and search_room(len(raw)): capped = [False] results = [ - _import_lines(reading, capped) if skipped else result + import_lines(reading, capped) if skipped else result for reading, result, skipped in zip(readings, results, refused) ] # An import past the colons tried isn't followed: a cut, never silent. diff --git a/src/fi/alk/harness/import_lines.py b/src/fi/alk/harness/import_lines.py new file mode 100644 index 000000000..518f06588 --- /dev/null +++ b/src/fi/alk/harness/import_lines.py @@ -0,0 +1,246 @@ +"""The import lines of code the walk didn't parse, found by reading its text. + +A parse costs time in proportion to the file and holds the conversation up, so the walk spends +only a total of them. What it can't parse it reads here instead: strings and comments are taken +out, then the lines that are imports are read, with no tree and no backtracking. Every step is +one pass over the text (a regular expression whose pieces can't overlap, or a split), so the cost +follows the size of the file, whatever it holds. + +A line search is not a parse. It misses an import built at run time (``importlib.import_module``, +``__import__``, ``exec``) and a name only a decoded codec spells. Where it can't be sure what it +read, it says so (``capped``) and the caller cuts the reading rather than trust the result. +""" + +from __future__ import annotations + +import re + +# The most of one file the walk line-searches, and of all the files it does: a file or a walk +# over these is a cut, not a search. +LARGEST_SEARCHED = 4 * 1024 * 1024 +SEARCH_TOTAL = 8 * 1024 * 1024 +# How many colons of a compound statement's header the search tries as the start of its body. +MOST_COLONS_TRIED = 8 + +# A dotted name. The pieces are split by a literal dot, so nothing in it can match two ways. +_NAME = re.compile(r"(?!\d)\w+(?:\.(?!\d)\w+)*") +# A backslash and what it escapes, which no string, comment or quote below then has to read. +_ESCAPE = re.compile(r"\\[^\n]") +# A comment, or a string: a triple-quoted one over any lines, or one that stays on its line +# (a continued line is "\x01\n" by then). A string that is never closed matches nothing, so its +# quote stays in the text, which says the search can't be sure. Each piece is a run of one kind +# of character and then the character that ends it, or a lazy single-character run, so a failed +# match costs the rest of its line (the rest of the file for a triple quote that has no end, +# which no later one can then have either) and none of it is tried again. +_STRING_OR_COMMENT = re.compile( + r"#[^\n]*" + r"|'''.*?'''" + r'|""".*?"""' + r"|'[^'\n\x01]*(?:\x01\n[^'\n\x01]*)*'" + r'|"[^"\n\x01]*(?:\x01\n[^"\n\x01]*)*"', + re.DOTALL, +) +_COMPOUND_START = re.compile( + r"(?:if|elif|else|for|while|try|except|finally|with|async|def|class|match|case)\b" +) + + +def _code(content: str, capped: list[bool] | None) -> str: + """``content`` with its comments and strings taken out and its continued lines joined. + + Given ``capped``, its one item is set when a quote is left over: a string never closed, so + what follows it was read as code that may be string.""" + content = content.replace("\r\n", "\n").replace("\r", "\n") + content = _ESCAPE.sub("\x00\x00", content.removeprefix("\ufeff")) + # A backslash before a line end: the line goes on. Kept as a marker until the strings and + # comments are out, since a comment ends at the line, and a string goes on over it. + content = content.replace("\\\n", "\x01\n") + code = _STRING_OR_COMMENT.sub(" ", content) + if capped is not None and ("'" in code or '"' in code): + capped[0] = True + return code.replace("\x01\n", " ") + + +def _statements(code: str, capped: list[bool] | None, most_colons: int) -> list[str]: + """The pieces of one line of code a statement can start at: each ``;`` segment, and for a + segment that opens a compound statement (``if True: import x``) the text after each of its + first ``most_colons`` colons. A piece that is no import is skipped by the caller. + Given ``capped``, its one item is set when a segment has more colons than are tried and an + ``import`` after the last one tried: that import isn't searched, so the caller cuts.""" + pieces: list[str] = [] + for segment in code.split(";"): + segment = segment.strip() + pieces.append(segment) + if _COMPOUND_START.match(segment): + at = -1 + for _ in range(most_colons): + at = segment.find(":", at + 1) + if at < 0: + break + pieces.append(segment[at + 1 :].strip()) + else: + rest = segment[at + 1 :] + if capped is not None and ":" in rest and "import" in rest: + capped[0] = True + return pieces + + +def _name(item: str) -> str | None: + """The dotted name an import item starts with (``a.b`` for ``a . b as c``), or None.""" + *head, tail = item.split(".") + words = tail.split() + if not words or (len(words) > 1 and words[1] != "as"): + return None + parts = [] + for part in head: + word = part.split() + if len(word) != 1: + return None + parts.append(word[0]) + name = ".".join([*parts, words[0]]) + return name if _NAME.fullmatch(name) else None + + +def _is_list_of_names(text: str) -> bool: + """Whether ``text`` is only what a parenthesised import list holds: names, ``as`` aliases and + commas. A line of anything else isn't part of one.""" + for item in text.split(","): + words = item.split() + if len(words) == 3: + if words[1] != "as": + return False + elif len(words) > 1: + return False + if words and not _NAME.fullmatch(words[0]): + return False + return True + + +def _import_keyword(text: str) -> int: + """Where ``text`` has the word ``import``, or -1: not part of a longer name, so + ``importlib`` and ``import_module`` aren't it, but ``.import`` is.""" + at = text.find("import") + while at >= 0: + end = at + 6 + if (at == 0 or not (text[at - 1].isalnum() or text[at - 1] == "_")) and ( + end == len(text) or not (text[end].isalnum() or text[end] == "_") + ): + return at + at = text.find("import", at + 1) + return -1 + + +def import_lines( + content: str, + capped: list[bool] | None = None, + most_colons: int = MOST_COLONS_TRIED, +) -> list[tuple[int, str]]: + """The names a line search finds ``content`` importing, each with its level, in the shape a + parse gives: ``import a.b, c as d`` and ``from .x import y`` / ``from a.b import c`` at the + start of a line (indented or not), after a ``;``, or after a compound statement's ``:``, + however the words are spaced, with a line continued by a backslash joined and a + parenthesised list read over its lines. Strings and comments hold no import. + + It misses an import built at run time (``importlib.import_module(name)``, ``__import__``) + and a name only a decoded codec spells. Given ``capped``, its one item is set where the + search can't be sure what it read, and the caller cuts rather than trust the result: a + string never closed, a parenthesised list that isn't one or never ends, or an import past the + colons tried (``_statements``). + """ + names: list[tuple[int, str]] = [] + lines = _code(content, capped).split("\n") + at = 0 + while at < len(lines): + line = lines[at] + at += 1 + if "import" not in line: + continue + for piece in _statements(line, capped, most_colons): + if piece.startswith("import") and piece[6:7].isspace(): + for item in piece[6:].split(","): + if name := _name(item): + names.append((0, name)) + elif piece.startswith("from") and (piece[4:5].isspace() or piece[4:5] == "."): + rest = piece[4:] + where = _import_keyword(rest) + head = "".join(rest[:where].split()) if where >= 0 else "" + level = len(head) - len(head.lstrip(".")) + module = head[level:] + if not head or (module and not _NAME.fullmatch(module)): + continue + tail = rest[where + 6 :] + if module: + names.append((level, module)) + if "(" in tail: + tail = tail.split("(", 1)[1] + if ")" in tail: + tail = tail.split(")", 1)[0] + else: + # The list goes on over the next lines, up to the one that closes it. A + # line that isn't names isn't part of it: the list was never closed, and + # the line is read as the code it is. + tail, at = _rest_of_list(tail, lines, at, capped) + for item in tail.split(","): + if name := _name(item): + names.append((level, f"{module}.{name}".lstrip("."))) + return names + + +def _rest_of_list( + tail: str, lines: list[str], at: int, capped: list[bool] | None +) -> tuple[str, int]: + """The text of a parenthesised import list that goes on over ``lines`` from ``at``, and where + reading resumes. A line that closes it is cut at its ``)``, and what follows is read as a + line of its own.""" + held = [tail] + while at < len(lines): + line = lines[at] + closes = ")" in line + inside = line.split(")", 1)[0] if closes else line + if not _is_list_of_names(inside): + break + held.append(inside) + if closes: + lines[at] = line.split(")", 1)[1] + return " , ".join(held), at + at += 1 + else: + pass + if capped is not None: + capped[0] = True + return " , ".join(held), at + + +# The two below belong with the search: the walk reads a script's import lines the same way, from +# its text with the comments taken out, and this module has no import of the scan's. +# Strings are matched first and kept, so "https://..." or "src/**/*.ts" isn't read as a comment. +# An unclosed one ends at its line or the file: a start that could fail makes a customer's odd file +# cost the square of its size, which holds up the build's conversation until it's done. +# A regex literal is kept the same way, so the "/*" in /\/*$/ doesn't hide the rest of the file. A +# slash where a value is expected starts one: after punctuation or at a line's start, never after a +# name or a number, where it divides. Its closing slash must be followed as a regex's is, by flags +# and then no name, quote, "/" or "*": a division that starts a line, or a "/" in JSX text, then +# reads as division, not as a literal that eats a string's opening quote. +# Such a start can fail, so a class holds no "[" and the rest no "]", and a literal that meets one +# is kept to its line's end: a failed literal then can't run over the classes of the slashes after +# it, which would make the line cost its square, and the quotes and backticks it holds can't open a +# string or a template that swaps code and strings for the rest of the file. The "]" also stops a +# literal that runs on through escaped line ends, which would cost the square of their number. +# Kept to its line's end, such a literal can take in a template's opening backtick later on that +# line too; a "/*" inside that template then reads as a comment. That trade is the price of a line +# that can't run away. +SCRIPT_COMMENT_OR_STRING = re.compile( + r"""("(?:\\.|[^"\\\n])*(?:"|(?=\n)|\\?\Z)|'(?:\\.|[^'\\\n])*(?:'|(?=\n)|\\?\Z)|`(?:\\.|[^`\\])*(?:`|\\?\Z)""" + r"""|(?])[ \t]*/(?![/*])(?:\\.|\[(?:\\.|[^\]\[\\\n])*(?:\]|(?=\n)|\\?\Z)|[^/\\\n\[\]])*""" + r"""(?:/(?=[dgimsuvy]*(?![\w$/*"'`]))|(?=\n)|\\?\Z|(?:\[(?=(?:\\.|[^\]\[\\\n])*\[)|\])[^\n]*))""" + r"""|//[^\n]*|/\*.*?(?:\*/|\Z)""", + re.DOTALL, +) + + +def script_kept(hit: re.Match[str]) -> str: + kept = hit.group(1) or "" + # A lone apostrophe, as in JSX text, isn't a string, so it can't protect a comment after it. + if kept.startswith("'") and not kept.endswith("'"): + return kept.split("//", 1)[0] + return kept diff --git a/tests/harness/test_agent_type_refusal.py b/tests/harness/test_agent_type_refusal.py index 7ef893642..577792e99 100644 --- a/tests/harness/test_agent_type_refusal.py +++ b/tests/harness/test_agent_type_refusal.py @@ -6353,7 +6353,7 @@ def test_files_read_two_ways_base_parsed_are_still_parsed(tmp_path): assert not stopped and {repo / "common" / "web.py", repo / "common" / "page.py"} <= found -@pytest.mark.parametrize("where", ["own-py", "imported-py", "imported-script", "grew-since"]) +@pytest.mark.parametrize("where", ["own-py", "imported-py", "grew-since"]) def test_a_file_over_the_read_limit_says_the_walk_stopped(tmp_path, monkeypatch, where): from fi.alk.harness import credentials @@ -6365,9 +6365,6 @@ def test_a_file_over_the_read_limit_says_the_walk_stopped(tmp_path, monkeypatch, elif where == "imported-py": (repo / "agents" / "a" / "main.py").write_text("import common.big\n") (repo / "common" / "big.py").write_bytes(big) - elif where == "imported-script": - (repo / "agents" / "a" / "index.js").write_text('import "./big.js";\n') - (repo / "agents" / "a" / "big.js").write_bytes(b'import "../../common/web.py";\n' + b"//" + b"x" * 1_000_100 + b"\n") else: (repo / "agents" / "a" / "main.py").write_bytes(big) real = credentials._plain_file @@ -6380,6 +6377,20 @@ def test_a_file_over_the_read_limit_says_the_walk_stopped(tmp_path, monkeypatch, assert stopped +def test_a_script_over_the_read_limit_is_matched_not_left_unread(tmp_path): + # The save's walk reaches a script past the read cap, up to the search cap, and matches it. + repo = _tree(tmp_path / "repo", {"common/__init__.py": "", "common/web.py": ""}).resolve() + (repo / "agents" / "a").mkdir(parents=True) + (repo / "agents" / "a" / "index.js").write_text('import "./big.js";\n') + (repo / "agents" / "a" / "big.js").write_bytes( + b'import "../../common/web.py";\n' + b"//" + b"x" * 1_000_100 + b"\n" + ) + + found, stopped = _save_walk(repo) + assert repo / "common" / "web.py" in found + assert not stopped + + def test_a_large_data_file_a_script_imports_doesnt_say_stopped(tmp_path): repo = _tree(tmp_path / "repo", {"agents/a/index.js": 'import data from "./data.json";\n'}).resolve() (repo / "agents" / "a" / "data.json").write_bytes(b"[" + b"1," * 600_000 + b"1]") diff --git a/tests/harness/test_browser_scan_cut.py b/tests/harness/test_browser_scan_cut.py index 3810aac1f..a1270e074 100644 --- a/tests/harness/test_browser_scan_cut.py +++ b/tests/harness/test_browser_scan_cut.py @@ -10,6 +10,7 @@ import json import logging import os +import time from pathlib import Path import pytest @@ -825,7 +826,7 @@ def _padded(root: Path, **more: str) -> Path: def test_a_file_the_parse_total_skipped_is_line_searched_and_no_cut(tmp_path, caplog): - # Dileep's C2: z_main.py is never parsed, so common.util used to go unfollowed. + # z_main.py is never parsed, so common.util used to go unfollowed. repo = _padded(tmp_path / "source", **{"common/util.py": "import playwright\n"}) with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) @@ -905,7 +906,7 @@ def test_targets_left_in_the_queue_by_the_import_limit_are_a_cut(tmp_path, monke ["x = 1; import common.util", "if True: import common.util", "x = 1;import common.util"], ) def test_an_import_after_a_semicolon_or_a_colon_is_line_searched_and_no_cut(tmp_path, line): - # Dileep's padding keeps z_main.py from being parsed; its import isn't at the line's start. + # The padding keeps z_main.py from being parsed; its import isn't at the line's start. repo = _padded( tmp_path / "source", **{"agents/a/z_main.py": f"{line}\n", "common/util.py": "import playwright\n"}, @@ -960,6 +961,202 @@ def read_counted(size=-1): assert read[0] <= agent_type_check._LARGEST_UNREAD_SEARCHED +# A code module over the 1,000,000 bytes a file is read at, made of nothing that imports. +_BIG_DATA = "WORDS = [\n" + ' "abcdefghijklmnopqrstuvwxyz",\n' * 55_000 + "]\n" + + +def test_an_imported_code_file_over_the_read_cap_is_line_searched_and_no_cut(tmp_path): + assert 1_800_000 < len(_BIG_DATA) < 1_900_000 + repo = _tree( + tmp_path / "source", + { + "agents/a/main.py": "import openai\nimport common.data\n", + "common/__init__.py": "", + "common/data.py": _BIG_DATA, + }, + ) + assert browser_scan(repo, "agents/a") == ([], frozenset()) + answer = _submitter(tmp_path / "out", repo, "agents/a")(_reading("chat")) + assert not answer.get("is_error"), answer + + +def test_a_big_module_that_imports_is_followed_through_its_import_lines(tmp_path): + repo = _tree( + tmp_path / "source", + { + "agents/a/main.py": "import common.big\n", + "common/__init__.py": "", + "common/big.py": "import common.deep\n" + _BIG_DATA, + "common/deep.py": "import playwright\n", + }, + ) + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + + +def test_an_imported_code_file_over_the_search_cap_is_still_a_cut(tmp_path): + big = "# padding\n".ljust(agent_type_check._LARGEST_UNREAD_SEARCHED + 1, "#") + repo = _tree( + tmp_path / "source", + { + "agents/a/main.py": "import common.data\n", + "common/__init__.py": "", + "common/data.py": big, + }, + ) + assert "import_walk" in browser_scan(repo, "agents/a")[1] + + +def test_an_imported_code_file_over_the_read_cap_costs_the_search_total(tmp_path, monkeypatch): + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", len(_BIG_DATA) - 1) + repo = _tree( + tmp_path / "source", + { + "agents/a/main.py": "import common.data\n", + "common/__init__.py": "", + "common/data.py": "import playwright\n" + _BIG_DATA, + }, + ) + assert "import_walk" in browser_scan(repo, "agents/a")[1] + + +@pytest.mark.parametrize( + "line, more", + [ + pytest.param("import os, \\\ncommon.util\n", {}, id="list-continued"), + pytest.param("from common import \\\nutil\n", {}, id="from-continued"), + pytest.param( + "from..import pkg\n", + {"agents/pkg/__init__.py": "import playwright\n"}, + id="no-space-around-import", + ), + pytest.param( + "from..sibling import util\n", + {"agents/sibling/__init__.py": "", "agents/sibling/util.py": "import playwright\n"}, + id="no-space-after-from", + ), + pytest.param('x = "#"; import common.util\n', {}, id="hash-in-string"), + pytest.param("x = 1\rimport common.util\r", {}, id="cr-line-ends"), + pytest.param("import\tcommon.util\n", {}, id="tab"), + pytest.param("import os as o, common.util as u\n", {}, id="as-list"), + pytest.param("from common import (os as o,\n util)\n", {}, id="paren-list"), + pytest.param( + "'''\nfrom x import (\n'''\nimport common.util\n", {}, id="open-paren-in-docstring" + ), + ], +) +def test_a_legal_import_form_is_line_searched_and_no_cut(tmp_path, line, more): + repo = _padded( + tmp_path / "source", + **{"agents/a/z_main.py": line, "common/util.py": "import playwright\n", **more}, + ) + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + + +@pytest.mark.parametrize( + "text", + [ + pytest.param("from x import (\nimport common.util\n", id="unclosed-paren-list"), + pytest.param( + "import common.util\nfrom x import (\n a,\n", id="paren-list-open-at-the-end" + ), + pytest.param("import common.util\n'''\nimport os\n", id="unclosed-triple-quote"), + pytest.param("x = 'it\nimport common.util\n", id="unclosed-quote"), + ], +) +def test_a_line_search_that_cant_be_sure_is_a_cut(tmp_path, text): + repo = _padded( + tmp_path / "source", + **{"agents/a/z_main.py": text, "common/util.py": "import playwright\n"}, + ) + assert browser_scan(repo, "agents/a")[1] == frozenset({"import_walk"}) + + +@pytest.mark.parametrize( + "line", + ["from" + " " * 100_000 + "x", " " * 100_000, "from ." + " " * 100_000 + "x", "(" * 100_000], + ids=["from-spaces", "spaces", "from-dot-spaces", "parens"], +) +def test_a_100_kb_line_in_an_unparsed_file_doesnt_hold_the_search_up(tmp_path, line): + repo = _padded( + tmp_path / "source", + **{ + "agents/a/z_main.py": f"{line}\nimport common.util\n", + "common/util.py": "import playwright\n", + }, + ) + started = time.perf_counter() + assert browser_scan(repo, "agents/a") == (["playwright"], frozenset()) + assert time.perf_counter() - started < 5 + + +def test_a_py_file_of_a_color_name_is_no_cut(tmp_path): + # "steel" is a browser package; "steelblue" isn't it. + colors = "BLUE = 'steelblue'\n" * ((LARGEST_PARSED_MODULE // 18) + 100) + assert len(colors) > LARGEST_PARSED_MODULE + repo = _tree(tmp_path / "a", {"main.py": "import openai\n", "colors.py": colors}) + assert browser_scan(repo) == ([], frozenset()) + + +@pytest.mark.parametrize( + "name", ["import steel\n", "x = steel.Client()\n", "'steel'\n", "import playwright\n"] +) +def test_a_py_file_over_the_read_size_that_names_a_package_whole_is_a_cut(tmp_path, name): + big = name + "# x\n" * (LARGEST_PARSED_MODULE // 4 + 100) + assert len(big) > LARGEST_PARSED_MODULE + repo = _tree(tmp_path / "a", {"main.py": "import openai\n", "big.py": big}) + assert browser_scan(repo)[1] == frozenset({"file_size"}) + + +def test_a_script_over_the_read_size_that_names_no_marker_is_no_cut(tmp_path): + widget = "window._selenium = window.playwright || 1;\n" * 30_000 + assert len(widget) > agent_type_check._LARGEST_CODE_FILE + repo = _tree(tmp_path / "a", {"main.py": "import openai\n", "widget.js": widget}) + assert browser_scan(repo) == ([], frozenset()) + + +@pytest.mark.parametrize("marker", ["@playwright/mcp", "computer_use_preview", "ComputerUse"]) +def test_a_script_over_the_read_size_that_holds_a_tool_marker_is_a_cut(tmp_path, marker): + widget = f"const tool = '{marker}';\n" + "var a = 1;\n" * 100_000 + assert len(widget) > agent_type_check._LARGEST_CODE_FILE + repo = _tree(tmp_path / "a", {"main.py": "import openai\n", "widget.js": widget}) + assert browser_scan(repo)[1] == frozenset({"file_size"}) + + +def _cut_lines(caplog) -> list[str]: + return [ + record.getMessage() + for record in caplog.records + if record.getMessage().startswith("agent type scan cut by") + ] + + +def test_a_reading_whose_only_cut_is_the_walk_logs_a_cut_line(tmp_path, monkeypatch, caplog): + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", 8) + repo = _padded(tmp_path / "source", **{"common/util.py": "import playwright\n"}) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + assert browser_scan(repo, "agents/a") == ([], frozenset({"import_walk"})) + assert _cut_lines(caplog) == ["agent type scan cut by import_walk 1"] + + +def test_the_walk_cut_joins_the_scans_own_in_one_cut_line(tmp_path, monkeypatch, caplog): + monkeypatch.setattr(agent_type_check, "_WALKED_SEARCHED", 8) + repo = _padded( + tmp_path / "source", + **{"common/util.py": "import playwright\n", "agents/a/big.py": _OVERSIZED}, + ) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + _, cut = browser_scan(repo, "agents/a") + assert cut == frozenset({"file_size", "import_walk"}) + assert _cut_lines(caplog) == ["agent type scan cut by file_size 1, import_walk 1"] + + +def test_a_walk_with_no_cut_logs_no_cut_line(tmp_path, caplog): + repo = _padded(tmp_path / "source", **{"common/util.py": "x = 1\n"}) + with caplog.at_level(logging.WARNING, logger=agent_type_check.__name__): + assert browser_scan(repo, "agents/a") == ([], frozenset()) + assert _cut_lines(caplog) == [] + + def test_ci_runs_this_file(): workflows = ( Path(__file__).resolve().parents[2] / ".github" / "workflows" / "sdk-smoke.yml" diff --git a/tests/harness/test_import_lines.py b/tests/harness/test_import_lines.py new file mode 100644 index 000000000..b6a786870 --- /dev/null +++ b/tests/harness/test_import_lines.py @@ -0,0 +1,238 @@ +"""The line search reads the import lines of a Python file that was never parsed. + +It must find every legal spelling of an import, say so (``capped``) where it can't be sure, and +cost time in proportion to the text, whatever the text is. +""" + +from __future__ import annotations + +import time + +import pytest + +from fi.alk.harness.credentials import _script_imports, _without_type_imports +from fi.alk.harness.import_lines import import_lines + +_FORMS = [ + # Each row: source, names (level, name), whether the search says it can't be sure. + pytest.param("import os\n", [(0, "os")], False, id="plain"), + pytest.param("import a.b, c\n", [(0, "a.b"), (0, "c")], False, id="dotted-and-list"), + pytest.param("import a as b, c as d\n", [(0, "a"), (0, "c")], False, id="as-list"), + pytest.param("import a . b\n", [(0, "a.b")], False, id="spaces-around-the-dot"), + pytest.param( + "import os, \\\ncommon.util\n", [(0, "os"), (0, "common.util")], False, id="list-continued" + ), + pytest.param( + "import os, \\\n common.util, \\\n x\n", + [(0, "os"), (0, "common.util"), (0, "x")], + False, + id="list-continued-twice", + ), + pytest.param( + "from common import \\\nutil\n", + [(0, "common"), (0, "common.util")], + False, + id="from-continued", + ), + pytest.param("from.common import util\n", [(1, "common"), (1, "common.util")], False, id="no-space-after-from"), + pytest.param("from .import util\n", [(1, "util")], False, id="no-space-before-import"), + pytest.param("from.import util\n", [(1, "util")], False, id="no-space-either-side"), + pytest.param("from . import util\n", [(1, "util")], False, id="dot"), + pytest.param("from .. import util\n", [(2, "util")], False, id="two-dots"), + pytest.param("from ... import util\n", [(3, "util")], False, id="ellipsis"), + pytest.param("from . . import util\n", [(2, "util")], False, id="dots-apart"), + pytest.param("from ..a.b import c\n", [(2, "a.b"), (2, "a.b.c")], False, id="relative-dotted"), + pytest.param("from a . b import c\n", [(0, "a.b"), (0, "a.b.c")], False, id="from-spaced-dots"), + pytest.param( + "from a import b as c, d as e\n", + [(0, "a"), (0, "a.b"), (0, "a.d")], + False, + id="from-as-list", + ), + pytest.param( + "from a import (b as c,\n d)\n", + [(0, "a"), (0, "a.b"), (0, "a.d")], + False, + id="paren-list-over-lines", + ), + pytest.param( + "from a import (\n b, # why\n\n c,\n)\nimport z\n", + [(0, "a"), (0, "a.b"), (0, "a.c"), (0, "z")], + False, + id="paren-list-with-comments-and-blank-line", + ), + pytest.param( + "from a import (b); import z\n", [(0, "a"), (0, "a.b"), (0, "z")], False, id="after-paren" + ), + pytest.param("from a import *\n", [(0, "a")], False, id="star"), + pytest.param( + "from importlib import import_module\n", + [(0, "importlib"), (0, "importlib.import_module")], + False, + id="module-named-import-something", + ), + pytest.param("import\ta\n", [(0, "a")], False, id="tab-after-import"), + pytest.param("\timport\ta\n", [(0, "a")], False, id="tab-before-and-after"), + pytest.param("\x0cimport a\n", [(0, "a")], False, id="form-feed-before"), + pytest.param("import\x0ca\n", [(0, "a")], False, id="form-feed-after"), + pytest.param("from\ta\timport\tb\n", [(0, "a"), (0, "a.b")], False, id="tabs-in-from"), + pytest.param("from a import\x0cb\n", [(0, "a"), (0, "a.b")], False, id="form-feed-in-from"), + pytest.param("x = 1\rimport a\rimport b\r", [(0, "a"), (0, "b")], False, id="cr-line-ends"), + pytest.param("x = 1\r\nimport a\r\nimport b\r\n", [(0, "a"), (0, "b")], False, id="crlf"), + pytest.param("import a\\\r\n, b\r\n", [(0, "a"), (0, "b")], False, id="crlf-continued"), + pytest.param("import a\n", [(0, "a")], False, id="bom"), + pytest.param("import café\n", [(0, "café")], False, id="non-ascii-name"), + pytest.param("x = 1; import a\n", [(0, "a")], False, id="after-semicolon"), + pytest.param("if True: import a\n", [(0, "a")], False, id="after-colon"), + pytest.param("else: from . import x\n", [(1, "x")], False, id="after-else"), + # A comment or a string holds no import, and doesn't hide one. + pytest.param("import a # import b\n", [(0, "a")], False, id="comment-after"), + pytest.param("# import a\n", [], False, id="comment-only"), + pytest.param('x = "#"; import common.util\n', [(0, "common.util")], False, id="hash-in-string"), + pytest.param("x = '#'; import common.util\n", [(0, "common.util")], False, id="hash-in-single"), + pytest.param('x = "a;b"; import common.util\n', [(0, "common.util")], False, id="semicolon-in-string"), + pytest.param('x = ";"\nimport a\n', [(0, "a")], False, id="semicolon-string-alone"), + pytest.param('x = "\\"#"; import a\n', [(0, "a")], False, id="escaped-quote-then-hash"), + pytest.param("x = '\\''; import a\n", [(0, "a")], False, id="escaped-single-quote"), + pytest.param('x = "\\\\"; import a # "\n', [(0, "a")], False, id="escaped-backslash-ends-string"), + pytest.param('x = "ab\\\ncd"; import a\n', [(0, "a")], False, id="string-continued"), + pytest.param("# note \\\nimport a\n", [(0, "a")], False, id="comment-does-not-continue"), + pytest.param('x = "import b"\n', [], False, id="import-text-in-a-string"), + pytest.param('"""\nimport b\n"""\nimport a\n', [(0, "a")], False, id="docstring"), + pytest.param('"""doc"""; import a\n', [(0, "a")], False, id="import-after-docstring-close"), + pytest.param("'''\nfrom x import (\n'''\nimport a\n", [(0, "a")], False, id="unclosed-paren-in-docstring"), + pytest.param("x = '''a\\''''\nimport a\n", [(0, "a")], False, id="triple-ending-in-escaped-quote"), + pytest.param('x = "\'\'\'"\nimport a\n', [(0, "a")], False, id="triple-quotes-inside-a-string"), + pytest.param("# '''\nimport a\n", [(0, "a")], False, id="triple-quotes-inside-a-comment"), + pytest.param("import_module('x')\nimports = 1\nfrom_x = 1\n", [], False, id="not-keywords"), + pytest.param("raise X from e\n", [], False, id="raise-from"), + pytest.param("yield from f()\n", [], False, id="yield-from"), + # Where the search can't be sure, it says so, and still gives what it found. + pytest.param( + "from x import (\nimport common.util\n", + [(0, "x"), (0, "common.util")], + True, + id="unclosed-paren-list", + ), + pytest.param( + "from x import (\n a,\n", [(0, "x"), (0, "x.a")], True, id="paren-list-open-at-the-end" + ), + pytest.param( + "import a\n'''\nimport b\n", [(0, "a"), (0, "b")], True, id="unclosed-triple-quote" + ), + pytest.param("x = 'abc\nimport a\n", [(0, "a")], True, id="unclosed-single-quote"), + pytest.param( + "if a[1:2:3:4:5:6:7:8:9]: import common.util\n", [], True, id="past-the-colons-tried" + ), + pytest.param( + "if a[1:2:3:4:5:6:7:8:9]: x = 1\nimport os\n", [(0, "os")], False, id="colons-and-no-import" + ), +] + + +@pytest.mark.parametrize("source, names, unsure", _FORMS) +def test_each_form_is_found_or_the_search_says_it_cant_be_sure(source, names, unsure): + capped = [False] + assert import_lines(source, capped) == names + assert capped[0] is unsure + + +def test_an_unclosed_paren_list_doesnt_swallow_the_lines_after_it(): + source = "from x import (\n" + "".join(f"import m{at}\n" for at in range(50)) + capped = [False] + found = import_lines(source, capped) + assert [name for _, name in found if name.startswith("m")] == [f"m{at}" for at in range(50)] + assert capped[0] + + +def test_the_search_without_capped_still_runs(): + assert import_lines("from x import (\n") == [(0, "x")] + + +_N = 100_000 +# Each is built to 100 KB in a test, as one line. The bound is generous: what it checks is that +# the cost isn't the square or the cube of the line. +_LINES = { + "spaces": " " * _N, + "tabs": "\t" * _N, + "dots": "." * _N, + "open-parens": "(" * _N, + "close-parens": ")" * _N, + "backslashes": "\\" * _N, + "hashes": "#" * _N, + "colons": ":" * _N, + "semicolons": ";" * _N, + "commas": "," * _N, + "single-quotes": "'" * _N, + "double-quotes": '"' * _N, + "triple-single": "'''" * (_N // 3), + "triple-double": '"""' * (_N // 3), + "escaped-quotes": "'\\" * (_N // 2), + "escaped-double-quotes": '"\\' * (_N // 2), + "import-words": "import" * (_N // 6), + "from-words": "from" * (_N // 4), + "from-spaced": "from " * (_N // 5), + "import-spaced": "import " * (_N // 7), + "from-then-spaces": "from" + " " * _N + "x", + "from-dot-spaces": "from ." + " " * _N + "x", + "from-dots-then-spaces": "from " + ". " * (_N // 2) + "x", + "from-name-then-spaces": "from a" + " " * _N + "x", + "from-import-then-spaces": "from a import" + " " * _N + "(", + "import-then-spaces": "import a" + " " * _N + "!", + "import-name-dots": "import " + "a." * (_N // 2), + "import-commas": "import " + "a, " * (_N // 3), + "from-import-commas": "from a import (" + "b, " * (_N // 3), + "from-import-as": "from a import " + "b as " * (_N // 5), + "if-colons": "if x: " * (_N // 6), + "if-colons-then-import": "if " + ":" * _N + " import a", + "semicolon-imports": "import a;" * (_N // 9), + "near-matches": "from . import (" * (_N // 15), + "comment-then-import": "# " * (_N // 2) + "import a", + "string-then-import": '"' + "a" * _N + '"; import a', +} + + +@pytest.mark.parametrize("name", list(_LINES)) +def test_a_100_kb_line_is_searched_at_once(name): + line = _LINES[name] + assert len(line) >= _N - 20 + started = time.perf_counter() + import_lines(line, [False]) + import_lines(line + "\n" + line + "\nimport a\n", [False]) + assert time.perf_counter() - started < 0.5 + + +def test_a_line_of_50000_spaces_after_from_returns_at_once(): + started = time.perf_counter() + assert import_lines("from" + " " * 50_000 + "x") == [] + assert time.perf_counter() - started < 0.5 + + +def test_many_lines_that_each_open_a_paren_list_cost_in_proportion(): + # Every one of 20,000 lines could swallow the rest of the file looking for its ")". + source = "from a import (\n" * 20_000 + started = time.perf_counter() + capped = [False] + import_lines(source, capped) + assert time.perf_counter() - started < 0.5 + assert capped[0] + + +def test_a_megabyte_of_short_strings_is_searched_in_proportion(): + source = "x = ['a', 'b']\n" * 70_000 + "import a\n" + started = time.perf_counter() + assert import_lines(source) == [(0, "a")] + assert time.perf_counter() - started < 1.5 + + +@pytest.mark.parametrize("name", list(_LINES)) +def test_the_script_searches_take_a_100_kb_line_at_once(name): + line = _LINES[name] + started = time.perf_counter() + _script_imports(line) + _script_imports(line + "\n" + line + "\n") + _without_type_imports(line) + # The search is for the walk's scripts; a line holding the keywords is its worst text. + _script_imports("import type " + line) + _script_imports("from " + line) + assert time.perf_counter() - started < 2.0