fix(tui): non-local path completion speaks POSIX from a Windows host and honours the composer cwd
Desktop's in-process gateway runs on the user's OS. On a Windows host with an ssh/docker terminal backend, os.path arithmetic in _dir_listing_items turned the search dir into a backslash path (`ws` + `sub/` -> `ws\sub`, `/tmp/x/` -> `\tmp\x`) that the backend's POSIX listing script cannot resolve, so every non-local completion came back empty. The non-local branch now uses posixpath for join/normpath/dirname/basename/relpath — the backend always speaks POSIX regardless of the host. Also: complete.path takes params['cwd'] (the session cwd Desktop's composer sends) as the backend root when supplied instead of ignoring it on non-local backends, and the per-keystroke backend listing waits at most 3s instead of 10s so a slow remote does not pin a pool worker. Review follow-up on #113144.
This commit is contained in:
@@ -127,6 +127,12 @@ def test_remote_backend_completion_lists_the_backend_not_the_host(tmp_path, monk
|
||||
|
||||
assert texts == ["remote-dir/", "remote-only.txt"], texts
|
||||
assert calls and calls[0][1] == "remote-key"
|
||||
# Desktop's composer sends the session cwd; on a non-local backend it names the backend's directory.
|
||||
(remote / "elsewhere" / "other-dir").mkdir(parents=True)
|
||||
resp = server.handle_request({
|
||||
"id": "2", "method": "complete.path",
|
||||
"params": {"word": "ot", "session_id": "remote-sid", "cwd": "elsewhere"}})
|
||||
assert [it["text"] for it in resp["result"]["items"]] == ["other-dir/"]
|
||||
|
||||
|
||||
def test_remote_backend_completion_expands_tilde_on_the_backend(tmp_path, monkeypatch):
|
||||
@@ -145,6 +151,30 @@ def test_remote_backend_completion_expands_tilde_on_the_backend(tmp_path, monkey
|
||||
assert os.path.expanduser("~") not in calls[0][0]
|
||||
|
||||
|
||||
def test_remote_backend_completion_speaks_posix_from_a_windows_host(tmp_path, monkeypatch):
|
||||
"""A Windows gateway host (Desktop's in-process gateway) with an ssh/docker backend must still address the
|
||||
backend with POSIX paths: ``os.path`` arithmetic there turns ``ws`` + ``sub/`` into ``ws\\sub``, which the
|
||||
backend's listing script cannot resolve, so every non-local completion came back empty."""
|
||||
import ntpath
|
||||
import types
|
||||
|
||||
remote = tmp_path / "remote"
|
||||
(remote / "ws" / "sub" / "remote-dir").mkdir(parents=True)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "ssh")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "ws")
|
||||
windows_os = types.ModuleType("os")
|
||||
windows_os.__dict__.update(vars(os))
|
||||
windows_os.path, windows_os.sep = ntpath, "\\"
|
||||
monkeypatch.setattr(server, "os", windows_os)
|
||||
calls = _fake_remote_backend(monkeypatch, remote)
|
||||
|
||||
texts = [t for t, _, _ in _items("sub/re")]
|
||||
|
||||
assert texts == ["sub/remote-dir/"], texts
|
||||
assert calls[0][0].endswith(" sh ws/sub"), calls[0][0]
|
||||
|
||||
|
||||
def test_bare_at_still_shows_static_refs(tmp_path, monkeypatch):
|
||||
"""`@` alone should list the static references so users discover the
|
||||
available prefixes. (Unchanged behaviour; regression guard.)
|
||||
|
||||
@@ -166,7 +166,7 @@ def _backend_dir_entries(search_dir: str, session_key: str | None) -> list[tuple
|
||||
try:
|
||||
from tools.terminal_tool import terminal_tool
|
||||
result = json.loads(terminal_tool(
|
||||
f"sh -c {shlex.quote(script)} sh {shlex.quote(search_dir)}", task_id=session_key, timeout=10))
|
||||
f"sh -c {shlex.quote(script)} sh {shlex.quote(search_dir)}", task_id=session_key, timeout=3))
|
||||
except Exception:
|
||||
return []
|
||||
if result.get("error") or result.get("exit_code") not in (0, None):
|
||||
@@ -178,16 +178,19 @@ def _backend_dir_entries(search_dir: str, session_key: str | None) -> list[tuple
|
||||
def _dir_listing_items(root: str, word: str, path_part: str, prefix_tag: str, is_context: bool,
|
||||
session_key: str | None = None) -> list[dict]:
|
||||
"""Prefix-match entries of the directory ``path_part`` points at (max 30)."""
|
||||
import posixpath
|
||||
local = _effective_terminal_backend() == "local"
|
||||
# A non-local backend expands ``~`` itself: the gateway host's home is the wrong one.
|
||||
# A non-local backend expands ``~`` itself (the gateway host's home is the wrong one) and its listing
|
||||
# script speaks POSIX: from a Windows gateway host, os.path would hand it ``~\\src`` and list nothing.
|
||||
pth = os.path if local else posixpath
|
||||
expanded = (_normalize_completion_path(path_part) if local else path_part) if path_part else "."
|
||||
if expanded == "." or not expanded or expanded.endswith("/"):
|
||||
search_dir, match = (expanded or "."), ""
|
||||
else:
|
||||
search_dir, match = os.path.dirname(expanded) or ".", os.path.basename(expanded)
|
||||
if not (os.path.isabs(search_dir) or search_dir.startswith("~")):
|
||||
search_dir = os.path.join(root, search_dir)
|
||||
search_dir = os.path.normpath(search_dir)
|
||||
search_dir, match = pth.dirname(expanded) or ".", pth.basename(expanded)
|
||||
if not (pth.isabs(search_dir) or search_dir.startswith("~")):
|
||||
search_dir = pth.join(root, search_dir)
|
||||
search_dir = pth.normpath(search_dir)
|
||||
items: list[dict] = []
|
||||
if local:
|
||||
if not os.path.isdir(search_dir):
|
||||
@@ -202,13 +205,13 @@ def _dir_listing_items(root: str, word: str, path_part: str, prefix_tag: str, is
|
||||
continue
|
||||
if prefix_tag and (prefix_tag == "folder") != is_dir: # explicit `@folder:`/`@file:` skip the other kind
|
||||
continue
|
||||
full = os.path.join(search_dir, entry)
|
||||
rel = os.path.relpath(full, root).replace(os.sep, "/")
|
||||
full = pth.join(search_dir, entry)
|
||||
rel = pth.relpath(full, root).replace(os.sep, "/")
|
||||
suffix = "/" if is_dir else ""
|
||||
if is_context:
|
||||
text = f"@{prefix_tag or ('folder' if is_dir else 'file')}:{rel}{suffix}"
|
||||
elif word.startswith("~"):
|
||||
text = "~/" + os.path.relpath(full, os.path.expanduser("~") if local else "~") + suffix
|
||||
text = "~/" + pth.relpath(full, os.path.expanduser("~") if local else "~") + suffix
|
||||
else:
|
||||
text = ("./" if word.startswith("./") else "") + rel + suffix
|
||||
items.append(_item(text, "dir" if is_dir else "", entry + suffix))
|
||||
@@ -225,8 +228,9 @@ def _(rid, params: dict) -> dict:
|
||||
return _ok(rid, {"items": []})
|
||||
session = _sessions.get(params.get("session_id", ""))
|
||||
local = _effective_terminal_backend() == "local"
|
||||
# A non-local backend's cwd lives inside the target; the host cannot validate it, so take it as-is.
|
||||
root = _completion_cwd(params) if local else _terminal_task_cwd(session)
|
||||
# A non-local backend's cwd lives inside the target; the host cannot validate it, so take the composer's
|
||||
# session cwd (Desktop sends it) or the session's terminal cwd as-is.
|
||||
root = _completion_cwd(params) if local else (params.get("cwd") or _terminal_task_cwd(session))
|
||||
session_key = session.get("session_key") if session else None
|
||||
is_context = word.startswith("@")
|
||||
query = word[1:] if is_context else word
|
||||
|
||||
Reference in New Issue
Block a user