test: fix six Windows-host failures across sandbox, read_file, lark shim, skill review, and extension checks (#6787)

* test: fix five Windows-host failures across sandbox, read_file, lark shim, and extension checks

Windows dev hosts currently fail five backend test cases that CI (ubuntu)
never exercises:

- test_local_sandbox_command_timeout: the fixture forces a bare /bin/sh
  shell and the Windows dispatch spawns it directly (WinError 2), and
  make(1) is not a given on Windows. Stub both runners; the contract under
  test is the timeout marker assembly, not the runner.
- test_local_sandbox_provider_mounts: the Popen-capture mock delegated to
  the real spawn, which cannot execute a POSIX shell path on Windows.
  Capture at the runner seam instead and assert the rewritten path
  shape-agnostically (the Windows runners pass the command string, not a
  [shell, -c, command] list).
- test_read_file_tool_binary: text-mode write_text leaks CRLF into the
  fixture on Windows; write the mount fixture as bytes (#6042 follow-up).
- test_lark_shim_atomic: skip the exact 0o755/0o644 assertions on Windows
  hosts, whose stat mode approximates POSIX bits (#6257 pattern).
- test_extension_check_script: zipfile normalizes os.sep in entry names on
  Windows reads, so a writestr-built fixture can never carry a verbatim
  backslash entry there. Inject the hostile name into namelist unless the
  archive already stores it, pinning the guard contract identically.

On a Windows 11 host these five files go from five failed cases to green
(102 passed, ruff check/format clean). Two more Windows-host failures
(skill-review NUL probe, jina shared-deadline classification) need deeper
root-causing and are left for follow-ups.

* test(skill-review): review the committed skill tree, not local residue

The video-generation review test scans the live directory, so untracked
build residue inside skills/public/video-generation fails it: once bytecode
caching drops a scripts/__pycache__ directory into the tree (importing the
skill script writes one), skillscan's package-undecodable-script finding is
correct and the test can never pass again until the residue is deleted by
hand. Reproduced on Windows: a planted scripts/__pycache__ pyc fails the
test; deleting it makes it pass.

- Materialize the tracked skill tree (git ls-files) into a temp dir and
  review that, so the test reviews exactly the committed package.
- tests/skills/skill_loader.py disables bytecode caching around
  exec_module, so the root skill suite stops polluting the committed tree.

Follows up on #6786 (item 6).

* docs(test): state the argv-list contract on both sandbox runners

The Windows dispatch also passes the resolved command as an argv list, so
the string fallback was unreachable on every platform.

---------

Co-authored-by: xiaodu55 <xiaodu55@users.noreply.github.com>

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"}))
 

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")

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)
 

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"

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)],

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

tests/skills/skill_loader.py

@@ -3,6 +3,7 @@
 Skills live in skills/public/<name>/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