diff --git a/backend/tests/test_extension_check_script.py b/backend/tests/test_extension_check_script.py index e51cab8ae8..7291b0101e 100644 --- a/backend/tests/test_extension_check_script.py +++ b/backend/tests/test_extension_check_script.py @@ -133,7 +133,18 @@ def test_wheel_paths_cannot_escape_temporary_directory(tmp_path, monkeypatch, na def fail_if_extracted(*args, **kwargs): pytest.fail("Unsafe wheel paths reached extraction") + # zipfile normalizes os.sep in entry names on Windows, so a wheel built + # through writestr never carries a verbatim backslash name there. Inject + # the hostile name into namelist unless the archive already stores it, so + # the guard contract is exercised with identical bytes on every host. + real_namelist = ZipFile.namelist + + def namelist_with_hostile_entry(self): + stored = real_namelist(self) + return stored if name in stored else [*stored, name] + monkeypatch.setattr(ZipFile, "extractall", fail_if_extracted) + monkeypatch.setattr(ZipFile, "namelist", namelist_with_hostile_entry) with pytest.raises(ValueError, match="wheel path"): checker.check_wheel(tmp_path, wheel(tmp_path, files={name: "no"})) diff --git a/backend/tests/test_lark_shim_atomic.py b/backend/tests/test_lark_shim_atomic.py index 7b9fb48734..78e7c3e729 100644 --- a/backend/tests/test_lark_shim_atomic.py +++ b/backend/tests/test_lark_shim_atomic.py @@ -20,6 +20,7 @@ def test_install_shim_publishes_executable_files(tmp_path): assert [p.name for p in (tmp_path / "bin").glob("*.tmp")] == [] +@pytest.mark.skipif(os.name == "nt", reason="Windows stat mode approximates POSIX permission bits (0o644 for every regular file); exact 0o755/0o644 assertions are POSIX-bound") def test_install_shim_publishes_correct_permissions(tmp_path): """Launcher and shim must be executable (0o755), marker readable (0o644).""" launcher = install_shim(str(tmp_path), version="1.2.3") diff --git a/backend/tests/test_local_sandbox_command_timeout.py b/backend/tests/test_local_sandbox_command_timeout.py index 5d49ab8394..c3a55e11cc 100644 --- a/backend/tests/test_local_sandbox_command_timeout.py +++ b/backend/tests/test_local_sandbox_command_timeout.py @@ -87,8 +87,13 @@ def test_timeout_output_carries_authoritative_failure_marker(monkeypatch): """A timed-out command is a failed execution: the output must carry an exit marker so exit-status evidence (acceptance checklist) cannot read a partial passing summary as success.""" + canned = ("12 passed\n", "", 0, True) monkeypatch.setattr(LocalSandbox, "_get_shell", lambda self: "/bin/sh") - monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(lambda args, timeout, env=None: ("12 passed\n", "", 0, True))) + monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(lambda args, timeout, env=None: canned)) + # Windows hosts dispatch timed-out commands through the MSYS/PowerShell + # runner instead of the POSIX one; stub it too so the marker contract is + # hermetic and does not require make(1) or a resolvable shell path. + monkeypatch.setattr(LocalSandbox, "_run_windows_command", staticmethod(lambda *args, **kwargs: canned)) output = LocalSandbox("t").execute_command("make test", timeout=1) diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index f6b570157c..34b7874129 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -582,25 +582,30 @@ class TestMultipleMounts: ], ) - # Mock subprocess to capture the resolved command. The POSIX path runs - # commands via subprocess.Popen, so wrap that and still execute the real - # command. + # Capture the resolved command at the runner seam. POSIX and Windows + # hosts dispatch through different runners (the Windows MSYS branch + # spawns the shell directly, so a delegating Popen mock would try to + # execute a POSIX shell path that does not exist), and the contract + # under test is the rewritten command, not the execution. captured = {} - original_popen = __import__("subprocess").Popen - def mock_popen(*args, **kwargs): - if len(args) > 0: - captured["command"] = args[0] - return original_popen(*args, **kwargs) + def capture_posix(args, timeout, env=None): + captured["command"] = args + return ("hello\n", "", 0, False) + + def capture_windows(*args, **kwargs): + captured["command"] = args[0] if args else [] + return ("hello\n", "", 0, False) - monkeypatch.setattr("deerflow.sandbox.local.local_sandbox.subprocess.Popen", mock_popen) monkeypatch.setattr("deerflow.sandbox.local.local_sandbox.LocalSandbox._get_shell", lambda self: "/bin/sh") + monkeypatch.setattr("deerflow.sandbox.local.local_sandbox.LocalSandbox._run_posix_command", staticmethod(capture_posix)) + monkeypatch.setattr("deerflow.sandbox.local.local_sandbox.LocalSandbox._run_windows_command", staticmethod(capture_windows)) sandbox.execute_command("cat /mnt/data/test.txt") - # Verify the command received the resolved local path - command = captured.get("command", []) - assert isinstance(command, list) and len(command) >= 3 - assert str(data_dir) in command[2] + # Verify the command received the resolved local path. Both runners + # receive the argv list, so join it before asserting. + text = " ".join(captured.get("command", [])) + assert str(data_dir.as_posix()) in text def test_reverse_resolve_path_does_not_match_partial_prefix(self, tmp_path): foo_dir = tmp_path / "foo" diff --git a/backend/tests/test_read_file_tool_binary.py b/backend/tests/test_read_file_tool_binary.py index 826962c9b4..31f32a5eab 100644 --- a/backend/tests/test_read_file_tool_binary.py +++ b/backend/tests/test_read_file_tool_binary.py @@ -71,7 +71,7 @@ def test_read_file_tool_keeps_custom_mount_path_provider_owned(tmp_path, monkeyp runtime = _local_runtime(tmp_path) mounted = tmp_path / "mounted-code" mounted.mkdir() - (mounted / "notes.txt").write_text("first\nsecond\nthird", encoding="utf-8") + (mounted / "notes.txt").write_bytes(b"first\nsecond\nthird") sandbox = LocalSandbox( "local:t1", path_mappings=[PathMapping(container_path="/mnt/code-read", local_path=str(mounted), read_only=True)], diff --git a/backend/tests/test_skill_review_core.py b/backend/tests/test_skill_review_core.py index 1869c3750f..7c9be8d739 100644 --- a/backend/tests/test_skill_review_core.py +++ b/backend/tests/test_skill_review_core.py @@ -1,6 +1,7 @@ import io import json import stat +import subprocess import tempfile import time import zipfile @@ -19,8 +20,31 @@ from deerflow.skills.review.resource_graph import _extract_references CONTRACTS_DIR = Path(__file__).resolve().parents[2] / "contracts" / "skill_review" -def test_video_generation_runtime_credentials_pass_skill_review(): - skill_dir = Path(__file__).resolve().parents[2] / "skills" / "public" / "video-generation" +def test_video_generation_runtime_credentials_pass_skill_review(tmp_path): + """The committed video-generation skill passes the package review. + + The review materializes the tracked skill tree instead of scanning the + live directory: local build residue (e.g. a scripts/__pycache__ directory + left behind when a test imports the skill script) is not part of the + committed package and must not fail the review. + """ + repo_skill_dir = Path(__file__).resolve().parents[2] / "skills" / "public" / "video-generation" + tracked = subprocess.run( + ["git", "ls-files", "--", "."], + cwd=repo_skill_dir, + capture_output=True, + text=True, + check=True, + ).stdout.splitlines() + assert tracked, "git ls-files returned no tracked files" + + skill_dir = tmp_path / "video-generation" + for rel in tracked: + source = repo_skill_dir / rel + target = skill_dir / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(source.read_bytes()) + facts = analyze_skill_package(LocalDirectoryReader(skill_dir).read()) assert facts["summary"]["blockers"] == 0 diff --git a/tests/skills/skill_loader.py b/tests/skills/skill_loader.py index 8ef5c23856..07a90e8d0f 100644 --- a/tests/skills/skill_loader.py +++ b/tests/skills/skill_loader.py @@ -3,6 +3,7 @@ Skills live in skills/public//scripts/generate.py and are NOT a package, so tests load them via importlib. Tests then mock the module's `requests`. """ + import importlib.util import sys from pathlib import Path @@ -19,7 +20,16 @@ def load(skill_name: str): spec = importlib.util.spec_from_file_location(mod_name, path) module = importlib.util.module_from_spec(spec) sys.modules[mod_name] = module # standard pattern; lets the module resolve itself - spec.loader.exec_module(module) + # Bytecode caching would drop a scripts/__pycache__ directory into the + # committed skill tree, and the skill review scans the live directory, so + # local runs would permanently fail the package review with a + # package-undecodable-script finding on the .pyc. + previous_dont_write_bytecode = sys.dont_write_bytecode + sys.dont_write_bytecode = True + try: + spec.loader.exec_module(module) + finally: + sys.dont_write_bytecode = previous_dont_write_bytecode return module