From c7c7096c69cbee698240262ad45eb3f99bfe794d Mon Sep 17 00:00:00 2001 From: kent Date: Wed, 22 Jul 2026 11:04:10 +0700 Subject: [PATCH] fix: bind-mount local --target by default to avoid slow file-by-file import (#725) A local directory passed via --target was handed to the SDK LocalDir manifest entry, which copies the tree into the sandbox file-by-file at session.start(). On large repositories this stalls for a very long time with no report produced (issue #725). Default local_code targets to a read-only bind mount (applied at container-create time, effectively instant) instead of the file-by-file copy. An explicit mount: False still forces the copy. Also soften the STRIX_MAX_LOCAL_COPY_MB oversize guard from a hard error to a warning so a large --target reaches the fast path instead of being rejected at parse time. Preserves --mount, cloned-repository, and explicit-copy behavior. Adds tests/test_fast_local_import.py (incl. a bug-condition regression test) and updates tests/test_local_sources.py for the new default. --- strix/interface/main.py | 14 +++-- strix/interface/utils.py | 7 ++- tests/test_fast_local_import.py | 90 +++++++++++++++++++++++++++++++++ tests/test_local_sources.py | 23 +++++++-- 4 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 tests/test_fast_local_import.py diff --git a/strix/interface/main.py b/strix/interface/main.py index 2403200d..b75ddf63 100644 --- a/strix/interface/main.py +++ b/strix/interface/main.py @@ -644,11 +644,15 @@ Examples: details = "; ".join( f"{path} ({size / (1024 * 1024):.0f} MB)" for path, size in oversized ) - parser.error( - f"Local target too large to stream into the sandbox: {details}. " - f"The limit is {max_local_copy_mb} MB " - "(set STRIX_MAX_LOCAL_COPY_MB to change it). Re-run with " - "--mount to bind-mount the directory instead of copying it." + # Issue #725: do NOT refuse large local targets. They are + # bind-mounted read-only into the sandbox by default (applied at + # container-create time, no file-by-file copy), so a large tree no + # longer stalls the scan. Warn for visibility and proceed. + logger.warning( + "Large local target(s) detected: %s. Bind-mounting read-only " + "into the sandbox (no file-by-file copy); the scan starts " + "immediately. Set STRIX_MAX_LOCAL_COPY_MB to tune this check.", + details, ) return args diff --git a/strix/interface/utils.py b/strix/interface/utils.py index 27c10a46..b9abdfae 100644 --- a/strix/interface/utils.py +++ b/strix/interface/utils.py @@ -1225,11 +1225,16 @@ def collect_local_sources(targets_info: list[dict[str, Any]]) -> list[dict[str, workspace_subdir = details.get("workspace_subdir") if target_info["type"] == "local_code" and "target_path" in details: + # Local directory targets bind-mount read-only by default (issue + # #725): the SDK LocalDir entry copies a tree into the sandbox + # file-by-file, which stalls for hours on large repos. A bind mount + # is applied at container-create time and is effectively instant. + # An explicit ``mount: False`` still forces the file-by-file copy. local_sources.append( { "source_path": details["target_path"], "workspace_subdir": workspace_subdir, - "mount": bool(details.get("mount", False)), + "mount": bool(details.get("mount", True)), } ) diff --git a/tests/test_fast_local_import.py b/tests/test_fast_local_import.py new file mode 100644 index 00000000..cb6a91ea --- /dev/null +++ b/tests/test_fast_local_import.py @@ -0,0 +1,90 @@ +"""Regression tests for issue #725: local --target hangs on file-by-file import. + +Root cause: a local directory ``--target`` was handed to the SDK ``LocalDir`` +manifest entry, which copies the tree into the sandbox file-by-file at +``session.start()`` — hours-long on large repos. The fix bind-mounts local +targets read-only by default (fast, applied at container-create time). + +The bug-condition exploration test asserts the *negation* of the bug condition +C(X): a local_code target must resolve to a bind mount, not a copied LocalDir +entry. It fails on the unfixed code (confirming the bug) and passes after the +fix. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from agents.sandbox.entries import LocalDir + +from strix.interface.utils import collect_local_sources +from strix.runtime.session_manager import build_session_entries + + +if TYPE_CHECKING: + from pathlib import Path + + +def _local_target(target_path: str, workspace_subdir: str = "repo") -> dict[str, Any]: + """A plain ``--target `` target_info (no explicit --mount).""" + return { + "type": "local_code", + "details": {"target_path": target_path, "workspace_subdir": workspace_subdir}, + "original": target_path, + } + + +def test_bug_condition_local_target_is_bind_mounted_not_copied(tmp_path: Path) -> None: + """BUG-CONDITION EXPLORATION (issue #725). + + A local directory target must be configured as a bind mount, NOT a + file-by-file LocalDir copy. Expected to FAIL on unfixed code, where a plain + --target defaults to a LocalDir copy. + """ + (tmp_path / "app.py").write_text("print('hi')\n", encoding="utf-8") + + sources = collect_local_sources([_local_target(str(tmp_path))]) + entries, bind_mounts, _staged = build_session_entries(sources) + + # Negation of C(X): bind-mounted, and NOT a copied LocalDir entry. + assert entries == {}, "local --target must not be a file-by-file LocalDir copy" + assert [m["target"] for m in bind_mounts] == ["/workspace/repo"] + assert all(not isinstance(v, LocalDir) for v in entries.values()) + + +def test_local_target_bind_mount_is_read_only(tmp_path: Path) -> None: + sources = collect_local_sources([_local_target(str(tmp_path))]) + _entries, bind_mounts, _staged = build_session_entries(sources) + + assert bind_mounts == [ + { + "source": str(tmp_path.resolve()), + "target": "/workspace/repo", + "read_only": True, + } + ] + + +def test_explicit_copy_still_supported(tmp_path: Path) -> None: + """An explicit mount=False local source is still copied (escape hatch preserved).""" + target = _local_target(str(tmp_path)) + target["details"]["mount"] = False + + sources = collect_local_sources([target]) + entries, bind_mounts, _staged = build_session_entries(sources) + + assert bind_mounts == [] + assert isinstance(entries["repo"], LocalDir) + + +def test_repository_source_is_unaffected(tmp_path: Path) -> None: + """FR2: cloned repositories retain their copy behavior (not bind-mounted).""" + repo = { + "type": "repository", + "details": {"cloned_repo_path": str(tmp_path), "workspace_subdir": "clone"}, + } + sources = collect_local_sources([repo]) + entries, bind_mounts, _staged = build_session_entries(sources) + + assert bind_mounts == [] + assert isinstance(entries["clone"], LocalDir) diff --git a/tests/test_local_sources.py b/tests/test_local_sources.py index 22ed4b9b..378e888e 100644 --- a/tests/test_local_sources.py +++ b/tests/test_local_sources.py @@ -106,19 +106,32 @@ def test_find_oversized_disabled_for_non_positive_limit(tmp_path: Path, disabled assert find_oversized_local_targets(targets, max_bytes=disabled) == [] -def test_collect_local_sources_propagates_mount_flag() -> None: - copied = _local_target("/copied") - copied["details"]["workspace_subdir"] = "copied" +def test_collect_local_sources_defaults_local_code_to_mount() -> None: + # Issue #725: a local_code target with no explicit flag now bind-mounts by + # default (fast) rather than copying file-by-file. + default_target = _local_target("/default") + default_target["details"]["workspace_subdir"] = "default" mounted = _local_target("/mounted", mount=True) mounted["details"]["workspace_subdir"] = "mounted" - sources = collect_local_sources([copied, mounted]) + sources = collect_local_sources([default_target, mounted]) by_path = {s["source_path"]: s for s in sources} - assert by_path["/copied"]["mount"] is False + assert by_path["/default"]["mount"] is True assert by_path["/mounted"]["mount"] is True +def test_collect_local_sources_respects_explicit_copy() -> None: + # An explicit ``mount: False`` preserves the file-by-file copy escape hatch. + copied = _local_target("/copied") + copied["details"]["workspace_subdir"] = "copied" + copied["details"]["mount"] = False + + sources = collect_local_sources([copied]) + + assert sources[0]["mount"] is False + + def test_collect_local_sources_repository_is_never_mounted() -> None: repo = { "type": "repository",