From 0bf5c09f800049045b917e94747a32206d032c4d Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Wed, 9 Sep 2026 18:51:08 +0530 Subject: [PATCH 1/2] fix(interface): handle Windows drive-letter paths in --workspace-file A Windows drive letter (`C:\path\file`) has its only colon right after the letter. `resolve_workspace_files` and `_workspace_file_dest` each called `spec.rpartition(":")` independently and treated any colon as the `PATH:DEST` separator, so a bare Windows path like `C:\temp\wordlist.txt` was split into source "C" and destination "\temp\wordlist.txt" -- argument parsing then rejected it because no file named "C" exists. Add a shared `_split_workspace_spec` helper used by both call sites. It detects the drive-letter case (a single-letter `raw` immediately followed by `\` or `/` in what would be `dest`) and treats the whole spec as the path instead, leaving the explicit `PATH:DEST` form -- including one where PATH itself starts with a drive letter -- unaffected, since rpartition's rightmost split there lands on the real destination separator, not the drive letter's colon. Fixes #1257 --- strix/interface/utils.py | 23 +++++++++++++++++++---- tests/test_workspace_files.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/strix/interface/utils.py b/strix/interface/utils.py index e051a1856..b1e01c9da 100644 --- a/strix/interface/utils.py +++ b/strix/interface/utils.py @@ -1715,10 +1715,26 @@ def validate_config_file(config_path: str) -> Path: # sources, so a large file makes session bring-up slower. +def _split_workspace_spec(spec: str) -> tuple[str, str | None]: + """Split ``PATH[:DEST]`` into ``(path, dest)``, ``dest`` is ``None`` if absent. + + A Windows drive letter's colon (``C:\\path\\file``) is the only colon in a + bare path. ``rpartition`` still finds it, leaving a single-letter ``raw`` + with the rest of the path in what would be ``dest`` -- that split is + detected here and rejected, so the whole spec is returned as the path. + """ + raw, sep, dest = spec.rpartition(":") + if not sep or not dest.strip(): + return spec, None + if re.fullmatch(r"[A-Za-z]", raw) and dest[:1] in ("\\", "/"): + return spec, None + return raw, dest + + def _workspace_file_dest(spec: str, source: Path) -> str: """Return the workspace-relative destination declared by ``spec``.""" - _, sep, dest = spec.rpartition(":") - candidate = dest.strip() if sep and dest.strip() else source.name + _, dest = _split_workspace_spec(spec) + candidate = dest.strip() if dest and dest.strip() else source.name if candidate.startswith("/") or Path(candidate).is_absolute(): if not candidate.startswith("/workspace/"): raise ValueError( @@ -1748,8 +1764,7 @@ def resolve_workspace_files(specs: list[str] | None) -> list[dict[str, str]]: resolved: list[dict[str, str]] = [] seen: dict[str, str] = {} for spec in specs or []: - raw, sep, dest = spec.rpartition(":") - source_text = raw if sep and dest.strip() else spec + source_text, _ = _split_workspace_spec(spec) source = Path(source_text.strip()).expanduser() if not source.is_file(): raise ValueError(f"'{source}' is not an existing file") diff --git a/tests/test_workspace_files.py b/tests/test_workspace_files.py index 6415a09c1..45335ed28 100644 --- a/tests/test_workspace_files.py +++ b/tests/test_workspace_files.py @@ -2,12 +2,17 @@ from __future__ import annotations +import sys from typing import TYPE_CHECKING import pytest from strix.core.inputs import build_root_task -from strix.interface.utils import read_workspace_files, resolve_workspace_files +from strix.interface.utils import ( + _split_workspace_spec, + read_workspace_files, + resolve_workspace_files, +) if TYPE_CHECKING: @@ -40,6 +45,32 @@ def test_a_declared_destination_is_taken_relative_to_the_workspace( assert resolved[0]["workspace_path"] == "/workspace/specs/openapi.yaml" +@pytest.mark.parametrize( + ("spec", "expected"), + [ + (r"C:\temp\wordlist.txt", (r"C:\temp\wordlist.txt", None)), + ("C:/temp/wordlist.txt", ("C:/temp/wordlist.txt", None)), + (r"C:\temp\wordlist.txt:dest/file", (r"C:\temp\wordlist.txt", "dest/file")), + ], +) +def test_a_windows_drive_letter_is_not_mistaken_for_a_dest_separator( + spec: str, expected: tuple[str, str | None] +) -> None: + assert _split_workspace_spec(spec) == expected + + +@pytest.mark.skipif(sys.platform != "win32", reason="tmp_path is only drive-letter-shaped here") +def test_a_bare_windows_path_resolves_end_to_end(tmp_path: Path) -> None: + source = tmp_path / "wordlist.txt" + source.write_text("admin\n", encoding="utf-8") + + resolved = resolve_workspace_files([str(source)]) + + assert resolved == [ + {"source_path": str(source.resolve()), "workspace_path": "/workspace/wordlist.txt"} + ] + + def test_a_missing_file_is_rejected(tmp_path: Path) -> None: with pytest.raises(ValueError, match="not an existing file"): resolve_workspace_files([str(tmp_path / "nope.txt")]) From bdb92ad1617eb64b124180af825b3ccc5abe564c Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Wed, 9 Sep 2026 19:03:37 +0530 Subject: [PATCH 2/2] Preserve single-letter sources with an explicit /workspace/ destination A single-letter source declaring an absolute /workspace/... destination (a:/workspace/input.txt) has the exact same shape rpartition(:) produces for a Windows drive letter, so the drive-letter check from the previous commit wrongly claimed it too. Exclude dest values starting with /workspace/, since that explicit form's meaning is already established and must not change. Addresses review feedback from greptile-apps on #1285. --- strix/interface/utils.py | 13 ++++++++++--- tests/test_workspace_files.py | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/strix/interface/utils.py b/strix/interface/utils.py index b1e01c9da..99a0b0039 100644 --- a/strix/interface/utils.py +++ b/strix/interface/utils.py @@ -1720,13 +1720,20 @@ def _split_workspace_spec(spec: str) -> tuple[str, str | None]: A Windows drive letter's colon (``C:\\path\\file``) is the only colon in a bare path. ``rpartition`` still finds it, leaving a single-letter ``raw`` - with the rest of the path in what would be ``dest`` -- that split is - detected here and rejected, so the whole spec is returned as the path. + with the rest of the path in what would be ``dest``. That collides with a + real single-letter source that declares an absolute ``/workspace/...`` + destination (``a:/workspace/input.txt``), so the drive-letter case is only + taken when ``dest`` is not one of those: the explicit form keeps its + existing meaning either way. """ raw, sep, dest = spec.rpartition(":") if not sep or not dest.strip(): return spec, None - if re.fullmatch(r"[A-Za-z]", raw) and dest[:1] in ("\\", "/"): + if ( + re.fullmatch(r"[A-Za-z]", raw) + and dest[:1] in ("\\", "/") + and not dest.startswith("/workspace/") + ): return spec, None return raw, dest diff --git a/tests/test_workspace_files.py b/tests/test_workspace_files.py index 45335ed28..ce8136bc1 100644 --- a/tests/test_workspace_files.py +++ b/tests/test_workspace_files.py @@ -59,6 +59,23 @@ def test_a_windows_drive_letter_is_not_mistaken_for_a_dest_separator( assert _split_workspace_spec(spec) == expected +def test_a_single_letter_source_with_a_workspace_destination_is_not_a_drive_letter() -> None: + assert _split_workspace_spec("a:/workspace/input.txt") == ("a", "/workspace/input.txt") + + +def test_a_single_letter_file_can_declare_an_absolute_workspace_destination( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + (tmp_path / "a").write_text("x", encoding="utf-8") + monkeypatch.chdir(tmp_path) + + # A relative, single-letter spec: "a:/workspace/input.txt" is exactly the + # shape rpartition(":") could confuse for a Windows drive letter. + resolved = resolve_workspace_files(["a:/workspace/input.txt"]) + + assert resolved[0]["workspace_path"] == "/workspace/input.txt" + + @pytest.mark.skipif(sys.platform != "win32", reason="tmp_path is only drive-letter-shaped here") def test_a_bare_windows_path_resolves_end_to_end(tmp_path: Path) -> None: source = tmp_path / "wordlist.txt"