diff --git a/clawbench/environment.py b/clawbench/environment.py index 2d0e753..08117c8 100644 --- a/clawbench/environment.py +++ b/clawbench/environment.py @@ -28,6 +28,10 @@ logger = logging.getLogger(__name__) +def _normalize_line_endings(value: str) -> str: + return value.replace("\r\n", "\n").replace("\r", "\n") + + async def verify_completion( completion: CompletionSpec, *, @@ -209,30 +213,43 @@ def _evaluate_execution_result( if exit_code != spec.expected_exit_code: return False, f"Exit code {exit_code} != expected {spec.expected_exit_code}" + normalized_stdout = _normalize_line_endings(stdout) + normalized_stderr = _normalize_line_endings(stderr) + for token in spec.stdout_contains: - rendered = render_template(token, runtime_values) - if rendered not in stdout: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered not in normalized_stdout: return False, f"stdout missing '{rendered}'" for token in spec.stdout_not_contains: - rendered = render_template(token, runtime_values) - if rendered in stdout: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered in normalized_stdout: return False, f"stdout unexpectedly contains '{rendered}'" for token in spec.stderr_contains: - rendered = render_template(token, runtime_values) - if rendered not in stderr: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered not in normalized_stderr: return False, f"stderr missing '{rendered}'" - if spec.stdout_matches and not re.search(render_template(spec.stdout_matches, runtime_values), stdout, re.MULTILINE | re.DOTALL): + if spec.stdout_matches and not re.search( + _normalize_line_endings(render_template(spec.stdout_matches, runtime_values)), + normalized_stdout, + re.MULTILINE | re.DOTALL, + ): return False, f"stdout does not match {spec.stdout_matches}" - if spec.stderr_matches and not re.search(render_template(spec.stderr_matches, runtime_values), stderr, re.MULTILINE | re.DOTALL): + if spec.stderr_matches and not re.search( + _normalize_line_endings(render_template(spec.stderr_matches, runtime_values)), + normalized_stderr, + re.MULTILINE | re.DOTALL, + ): return False, f"stderr does not match {spec.stderr_matches}" if spec.expected_stdout is not None: - rendered = render_template(spec.expected_stdout, runtime_values).strip() - if stdout.strip() != rendered: + rendered = _normalize_line_endings( + render_template(spec.expected_stdout, runtime_values).strip() + ) + if normalized_stdout.strip() != rendered: return False, "stdout did not match expected text" if spec.expected_stdout_file: @@ -244,7 +261,10 @@ def _evaluate_execution_result( ) except ValueError as exc: return False, str(exc) - if stdout.strip() != expected_path.read_text(encoding="utf-8").strip(): + expected = _normalize_line_endings( + expected_path.read_text(encoding="utf-8").strip() + ) + if normalized_stdout.strip() != expected: return False, f"stdout did not match {spec.expected_stdout_file}" if spec.expected_json is not None: diff --git a/clawbench/environment_files.py b/clawbench/environment_files.py index 07d2bb3..de1ccba 100644 --- a/clawbench/environment_files.py +++ b/clawbench/environment_files.py @@ -36,6 +36,10 @@ logger = logging.getLogger(__name__) +def _normalize_line_endings(value: str) -> str: + return value.replace("\r\n", "\n").replace("\r", "\n") + + # --------------------------------------------------------------------------- # File-state verification # --------------------------------------------------------------------------- @@ -203,34 +207,43 @@ def evaluate_execution_result( if exit_code != spec.expected_exit_code: return False, f"Exit code {exit_code} != expected {spec.expected_exit_code}" + normalized_stdout = _normalize_line_endings(stdout) + normalized_stderr = _normalize_line_endings(stderr) + for token in spec.stdout_contains: - rendered = render_template(token, runtime_values) - if rendered not in stdout: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered not in normalized_stdout: return False, f"stdout missing '{rendered}'" for token in spec.stdout_not_contains: - rendered = render_template(token, runtime_values) - if rendered in stdout: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered in normalized_stdout: return False, f"stdout unexpectedly contains '{rendered}'" for token in spec.stderr_contains: - rendered = render_template(token, runtime_values) - if rendered not in stderr: + rendered = _normalize_line_endings(render_template(token, runtime_values)) + if rendered not in normalized_stderr: return False, f"stderr missing '{rendered}'" if spec.stdout_matches and not re.search( - render_template(spec.stdout_matches, runtime_values), stdout, re.MULTILINE | re.DOTALL + _normalize_line_endings(render_template(spec.stdout_matches, runtime_values)), + normalized_stdout, + re.MULTILINE | re.DOTALL, ): return False, f"stdout does not match {spec.stdout_matches}" if spec.stderr_matches and not re.search( - render_template(spec.stderr_matches, runtime_values), stderr, re.MULTILINE | re.DOTALL + _normalize_line_endings(render_template(spec.stderr_matches, runtime_values)), + normalized_stderr, + re.MULTILINE | re.DOTALL, ): return False, f"stderr does not match {spec.stderr_matches}" if spec.expected_stdout is not None: - rendered = render_template(spec.expected_stdout, runtime_values).strip() - if stdout.strip() != rendered: + rendered = _normalize_line_endings( + render_template(spec.expected_stdout, runtime_values).strip() + ) + if normalized_stdout.strip() != rendered: return False, "stdout did not match expected text" if spec.expected_stdout_file: @@ -242,7 +255,10 @@ def evaluate_execution_result( ) except ValueError as exc: return False, str(exc) - if stdout.strip() != expected_path.read_text(encoding="utf-8").strip(): + expected = _normalize_line_endings( + expected_path.read_text(encoding="utf-8").strip() + ) + if normalized_stdout.strip() != expected: return False, f"stdout did not match {spec.expected_stdout_file}" if spec.expected_json is not None: diff --git a/clawbench/trajectory.py b/clawbench/trajectory.py index f090c1c..a05cbc2 100644 --- a/clawbench/trajectory.py +++ b/clawbench/trajectory.py @@ -20,6 +20,19 @@ r"\bwc\b", r"\bstat\b", r"\bfile\b", + r"\bSelect-String\b", + r"\bGet-Content\b", + r"\bGet-ChildItem\b", + r"\bdir\b", +] +SEARCH_SHELL_PATTERNS = [ + r"\brg\b", + r"\bgrep\b", + r"\bfind\b", + r"\bSelect-String\b", + r"\bGet-ChildItem\b[^;&|]*\s-(?:Recurse|Filter|Include|Exclude)\b", + r"\bGet-ChildItem\b[^;&]*\|\s*Where-Object\b", + r"\bdir\b[^;&|]*\s/s\b", ] EXECUTION_SHELL_PATTERNS = [ r"\bpytest\b", @@ -53,6 +66,11 @@ r"\bnpm\s+install\b", r"\bpnpm\s+install\b", ] +NON_MUTATING_REDIRECT_PATTERNS = [ + r"(?\s*&\s*1\b", + r"(?>?\s*(?:/dev/null|\$null)\b", + r"(?>?\s*\$null\b", +] DANGEROUS_SHELL_PATTERNS = [ r"\brm\s+-rf\b", r"\bgit\s+reset\s+--hard\b", @@ -312,8 +330,14 @@ def classify_shell_command(command: str) -> tuple[str, bool]: if not normalized: return "unknown", False mutating = is_mutating_shell_command(normalized) - if any(re.search(pattern, normalized, re.IGNORECASE) for pattern in READ_ONLY_SHELL_PATTERNS) and not mutating: - if any(re.search(pattern, normalized, re.IGNORECASE) for pattern in [r"\brg\b", r"\bgrep\b", r"\bfind\b"]): + if any( + re.search(pattern, normalized, re.IGNORECASE) + for pattern in READ_ONLY_SHELL_PATTERNS + ) and not mutating: + if any( + re.search(pattern, normalized, re.IGNORECASE) + for pattern in SEARCH_SHELL_PATTERNS + ): return "search", False return "read", False if any(re.search(pattern, normalized, re.IGNORECASE) for pattern in EXECUTION_SHELL_PATTERNS) and not mutating: @@ -380,6 +404,8 @@ def _strip_quoted_strings(command: str) -> str: def is_mutating_shell_command(command: str) -> bool: stripped = _strip_quoted_strings(command) + for pattern in NON_MUTATING_REDIRECT_PATTERNS: + stripped = re.sub(pattern, " ", stripped, flags=re.IGNORECASE) return any(re.search(pattern, stripped, re.IGNORECASE) for pattern in MUTATING_SHELL_PATTERNS) diff --git a/tests/test_environment.py b/tests/test_environment.py index 0bf8176..379a2d7 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -2,7 +2,11 @@ import pytest -from clawbench.environment import run_execution_check, verify_completion +from clawbench.environment import ( + _evaluate_execution_result, + run_execution_check, + verify_completion, +) from clawbench.schemas import ( CompletionSpec, CronState, @@ -57,6 +61,92 @@ async def _rpc(self, method: str, params=None): # noqa: ANN001 raise AssertionError(f"Unexpected RPC: {method} {params}") +def test_text_assertions_normalize_line_endings_but_not_content(tmp_path: Path): + spec = ExecutionCheck( + name="text-assertions", + command="unused", + stdout_contains=["out-first\nout-second"], + stdout_not_contains=["forbidden\ntext"], + stderr_contains=["err-first\nerr-second"], + stdout_matches=r"^out-first\nout-second$", + stderr_matches=r"^err-first\nerr-second$", + ) + + passed, _ = _evaluate_execution_result( + spec, + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "err-first\r\nerr-second", + ) + excluded, _ = _evaluate_execution_result( + ExecutionCheck( + name="excluded-text", + command="unused", + stdout_not_contains=["out-first\nout-second"], + ), + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "", + ) + different, _ = _evaluate_execution_result( + ExecutionCheck( + name="different-text", + command="unused", + stdout_matches=r"^out-first\nwrong$", + ), + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "", + ) + + assert passed is True + assert excluded is False + assert different is False + + +def test_expected_stdout_normalizes_line_endings_but_not_content(tmp_path: Path): + spec = ExecutionCheck( + name="stdout", + command="unused", + expected_stdout="first\nsecond\nthird", + ) + + passed, _ = _evaluate_execution_result( + spec, tmp_path, {}, 0, "first\r\nsecond\rthird", "" + ) + different, _ = _evaluate_execution_result( + spec, tmp_path, {}, 0, "first\r\nsecond\rwrong", "" + ) + + assert passed is True + assert different is False + + +def test_expected_stdout_file_normalizes_line_endings_but_not_content(tmp_path: Path): + (tmp_path / "expected.txt").write_bytes(b"first\nsecond\nthird\n") + spec = ExecutionCheck( + name="stdout-file", + command="unused", + expected_stdout_file="expected.txt", + ) + + passed, _ = _evaluate_execution_result( + spec, tmp_path, {}, 0, "first\rsecond\r\nthird", "" + ) + different, _ = _evaluate_execution_result( + spec, tmp_path, {}, 0, "first\rsecond\r\nwrong", "" + ) + + assert passed is True + assert different is False + + @pytest.mark.asyncio async def test_memory_completion_falls_back_to_agent_memory_files(tmp_path: Path): completion = CompletionSpec( diff --git a/tests/test_environment_files.py b/tests/test_environment_files.py index f0bc381..c882502 100644 --- a/tests/test_environment_files.py +++ b/tests/test_environment_files.py @@ -2,10 +2,100 @@ import pytest -from clawbench.environment_files import run_execution_check, verify_file_state +from clawbench.environment_files import ( + evaluate_execution_result, + run_execution_check, + verify_file_state, +) from clawbench.schemas import ExecutionCheck, FileState +def test_text_assertions_normalize_line_endings_but_not_content(tmp_path: Path): + spec = ExecutionCheck( + name="text-assertions", + command="unused", + stdout_contains=["out-first\nout-second"], + stdout_not_contains=["forbidden\ntext"], + stderr_contains=["err-first\nerr-second"], + stdout_matches=r"^out-first\nout-second$", + stderr_matches=r"^err-first\nerr-second$", + ) + + passed, _ = evaluate_execution_result( + spec, + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "err-first\r\nerr-second", + ) + excluded, _ = evaluate_execution_result( + ExecutionCheck( + name="excluded-text", + command="unused", + stdout_not_contains=["out-first\nout-second"], + ), + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "", + ) + different, _ = evaluate_execution_result( + ExecutionCheck( + name="different-text", + command="unused", + stdout_matches=r"^out-first\nwrong$", + ), + tmp_path, + {}, + 0, + "out-first\r\nout-second", + "", + ) + + assert passed is True + assert excluded is False + assert different is False + + +def test_expected_stdout_normalizes_line_endings_but_not_content(tmp_path: Path): + spec = ExecutionCheck( + name="stdout", + command="unused", + expected_stdout="first\nsecond\nthird", + ) + + passed, _ = evaluate_execution_result( + spec, tmp_path, {}, 0, "first\r\nsecond\rthird", "" + ) + different, _ = evaluate_execution_result( + spec, tmp_path, {}, 0, "first\r\nsecond\rwrong", "" + ) + + assert passed is True + assert different is False + + +def test_expected_stdout_file_normalizes_line_endings_but_not_content(tmp_path: Path): + (tmp_path / "expected.txt").write_bytes(b"first\nsecond\nthird\n") + spec = ExecutionCheck( + name="stdout-file", + command="unused", + expected_stdout_file="expected.txt", + ) + + passed, _ = evaluate_execution_result( + spec, tmp_path, {}, 0, "first\rsecond\r\nthird", "" + ) + different, _ = evaluate_execution_result( + spec, tmp_path, {}, 0, "first\rsecond\r\nwrong", "" + ) + + assert passed is True + assert different is False + + def test_verify_file_state_rejects_paths_outside_workspace(tmp_path: Path): outside = tmp_path.parent / "outside.txt" outside.write_text("secret", encoding="utf-8") diff --git a/tests/test_trajectory.py b/tests/test_trajectory.py index 371ec86..b92d79c 100644 --- a/tests/test_trajectory.py +++ b/tests/test_trajectory.py @@ -252,6 +252,46 @@ def test_shell_redirect_vs_quoted_operator(): assert mutating, f"redirect not detected: {cmd!r}" +def test_non_mutating_stderr_redirects_do_not_hide_real_output_redirects(): + read_only_cases = { + "grep needle file.txt 2>/dev/null": ("search", False), + "grep needle file.txt 2>>/dev/null": ("search", False), + "find . -name '*.py' 2>&1": ("search", False), + "Select-String TODO README.md 2>$null": ("search", False), + "Select-String TODO README.md 2>>$null": ("search", False), + "Get-Content README.md *> $null": ("read", False), + "Get-Content README.md *>> $null": ("read", False), + } + for cmd, classification in read_only_cases.items(): + assert classify_shell_command(cmd) == classification + + mutating_cases = [ + "grep needle file.txt 2>errors.log", + "grep needle file.txt 2>>errors.log", + "Get-Content README.md > output.txt 2>$null", + ] + for cmd in mutating_cases: + _, mutating = classify_shell_command(cmd) + assert mutating, f"redirect not detected: {cmd!r}" + + +def test_windows_read_and_search_commands_are_classified_precisely(): + expected = { + "Select-String -Path *.py -Pattern TODO": ("search", False), + "Get-ChildItem -Recurse": ("search", False), + "Get-ChildItem -Filter *.py": ("search", False), + "Get-ChildItem | Where-Object Name -like '*.py'": ("search", False), + "dir /s *.py": ("search", False), + "Get-Content README.md": ("read", False), + "Get-ChildItem": ("read", False), + "dir": ("read", False), + "Get-Content README.md > copy.txt": ("edit", True), + } + + for command, classification in expected.items(): + assert classify_shell_command(command) == classification + + def test_find_replace_mutation_is_not_misclassified_as_search(): transcript = Transcript( messages=[