Skip to content

refactor: remove tunnel doctor preflight #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timeleft-- merged 1 commit into from
Jul 11, 2026
Merged

refactor: remove tunnel doctor preflight #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,16 @@ tunnel exposure. Keep `--sync-interval-seconds` at its default `0` when no
later autosync is wanted; a positive interval additionally starts the recurring
worker without repeating the initial sync.

Before starting the external tunnel, deployments can validate the same private
runtime and optionally the tunnel profile without exposing it:
Before starting the external tunnel, deployments can validate the private
runtime without exposing it:

```bash
fava-trails-tunnel preflight --data-repo /path/to/fava-trails-data --profile fava-trails --tunnel-doctor
fava-trails-tunnel preflight --data-repo /path/to/fava-trails-data --profile fava-trails
```

`preflight` starts only the loopback HTTP runtime, waits for `/healthz`, runs the
optional doctor, and always stops the private runtime before returning. It never
starts the external tunnel.
`preflight` starts only the loopback HTTP runtime, waits for `/healthz`, and
always stops the private runtime before returning. It never starts the external
tunnel.

The tunnel startup wait and `status` command consume `/healthz`; `status` exits
non-zero when the supervisor is running but its data is not ready:
Expand Down
3 changes: 1 addition & 2 deletions examples/tunnel-gateway/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ Run a non-exposing check before the service is started:
docker compose run --rm fava-tunnel \
fava-trails-tunnel preflight \
--data-repo "$FAVA_DATA_REPO" \
--profile "$FAVA_TUNNEL_PROFILE" \
--tunnel-doctor
--profile "$FAVA_TUNNEL_PROFILE"
```

The service performs one startup sync and sets the interval to zero, so it does
Expand Down
1 change: 0 additions & 1 deletion examples/tunnel-gateway/compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ services:
- --sync-on-start
- --sync-interval-seconds
- "0"
- --tunnel-doctor
29 changes: 2 additions & 27 deletions src/fava_trails/tunnel_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,18 +323,6 @@ def _start_http_runtime(
)


def _run_tunnel_doctor(config: GatewayConfig) -> None:
result = subprocess.run(
[config.tunnel_client, "doctor", "--profile", config.profile, "--explain"],
check=False,
text=True,
)
if result.returncode != 0:
raise subprocess.SubprocessError(
f"tunnel-client doctor failed for profile {config.profile!r}"
)


def _start_tunnel_client(config: GatewayConfig) -> subprocess.Popen:
return subprocess.Popen(
[config.tunnel_client, "run", "--profile", config.profile],
Expand Down Expand Up @@ -678,10 +666,6 @@ def handle_shutdown(signum, frame): # noqa: ARG001
_wait_for_health(config.health_url, http_process, timeout=args.ready_timeout)
print(" Private MCP runtime: ready")

if getattr(args, "tunnel_doctor", False):
_run_tunnel_doctor(config)
print(" tunnel-client doctor: ok")

tunnel_process = _start_tunnel_client(config)
if state_dir:
_write_metadata(
Expand Down Expand Up @@ -719,16 +703,13 @@ def handle_shutdown(signum, frame): # noqa: ARG001


def cmd_preflight(args: argparse.Namespace) -> int:
"""Validate the private runtime and optional tunnel profile without exposure."""
"""Validate the private runtime without external tunnel exposure."""
http_process: subprocess.Popen | None = None
try:
run_doctor = bool(getattr(args, "tunnel_doctor", False))
config = _load_gateway_config(args, require_tunnel_client=run_doctor)
config = _load_gateway_config(args, require_tunnel_client=False)
_check_port_available(config.host, config.port)
http_process = _start_http_runtime(config)
_wait_for_health(config.health_url, http_process, timeout=args.ready_timeout)
if run_doctor:
_run_tunnel_doctor(config)
print("FAVA Trails tunnel preflight: ok")
return 0
except (OSError, ValueError, subprocess.SubprocessError, TimeoutError) as exc:
Expand Down Expand Up @@ -779,8 +760,6 @@ def cmd_start(args: argparse.Namespace) -> int:
"--state-dir",
str(state_dir),
]
if getattr(args, "tunnel_doctor", False):
command.append("--tunnel-doctor")
if getattr(args, "sync_on_start", False):
command.append("--sync-on-start")
command.extend([
Expand Down Expand Up @@ -938,7 +917,6 @@ def build_parser() -> argparse.ArgumentParser:
_add_common_args(p_run)
p_run.add_argument("--tunnel-client", default="tunnel-client", help="Path to tunnel-client binary")
p_run.add_argument("--ready-timeout", type=float, default=20.0, help="Seconds to wait for local MCP readiness")
p_run.add_argument("--tunnel-doctor", action="store_true", help="Run tunnel-client doctor before starting the tunnel")
p_run.add_argument("--sync-on-start", action="store_true", help="Require one successful data repo sync before exposing the gateway")
p_run.add_argument("--sync-interval-seconds", type=float, default=DEFAULT_SYNC_INTERVAL_SECONDS, help=f"Seconds between data repo syncs; 0 disables tunnel-managed sync (default: {DEFAULT_SYNC_INTERVAL_SECONDS:g})")
p_run.add_argument("--sync-timeout-seconds", type=float, default=DEFAULT_SYNC_TIMEOUT_SECONDS, help=f"Seconds before a data repo sync is marked failed (default: {DEFAULT_SYNC_TIMEOUT_SECONDS:g})")
Expand All @@ -949,17 +927,14 @@ def build_parser() -> argparse.ArgumentParser:
_add_common_args(p_start)
p_start.add_argument("--tunnel-client", default="tunnel-client", help="Path to tunnel-client binary")
p_start.add_argument("--ready-timeout", type=float, default=20.0, help="Seconds to wait for local MCP readiness")
p_start.add_argument("--tunnel-doctor", action="store_true", help="Run tunnel-client doctor before starting the tunnel")
p_start.add_argument("--sync-on-start", action="store_true", help="Require one successful data repo sync before exposing the gateway")
p_start.add_argument("--sync-interval-seconds", type=float, default=DEFAULT_SYNC_INTERVAL_SECONDS, help=f"Seconds between data repo syncs; 0 disables tunnel-managed sync (default: {DEFAULT_SYNC_INTERVAL_SECONDS:g})")
p_start.add_argument("--sync-timeout-seconds", type=float, default=DEFAULT_SYNC_TIMEOUT_SECONDS, help=f"Seconds before a data repo sync is marked failed (default: {DEFAULT_SYNC_TIMEOUT_SECONDS:g})")
p_start.set_defaults(func=cmd_start)

p_preflight = subparsers.add_parser("preflight", help="Validate the private runtime without starting the external tunnel")
_add_common_args(p_preflight)
p_preflight.add_argument("--tunnel-client", default="tunnel-client", help="Path to tunnel-client binary")
p_preflight.add_argument("--ready-timeout", type=float, default=20.0, help="Seconds to wait for local MCP readiness")
p_preflight.add_argument("--tunnel-doctor", action="store_true", help="Run tunnel-client doctor after private readiness succeeds")
p_preflight.set_defaults(func=cmd_preflight)

p_stop = subparsers.add_parser("stop", help="Stop a detached gateway")
Expand Down
116 changes: 23 additions & 93 deletions tests/test_tunnel_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import argparse
import json
import subprocess
from pathlib import Path
from unittest.mock import MagicMock, patch

Expand Down Expand Up @@ -40,7 +39,6 @@ def _args(**overrides):
"mcp_path": DEFAULT_MCP_PATH,
"tunnel_client": "tunnel-client",
"ready_timeout": 0.1,
"tunnel_doctor": False,
"sync_on_start": False,
"state_dir": None,
"sync_interval_seconds": 0.0,
Expand Down Expand Up @@ -217,7 +215,7 @@ def test_disabled_sync_health_does_not_query_revision_state(tmp_path, monkeypatc
assert "remote_main" not in payload


def test_run_starts_http_and_runs_tunnel_without_doctor_or_autosync_by_default(tmp_path, monkeypatch):
def test_run_starts_http_and_runs_tunnel_without_autosync_by_default(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
args = _args(data_repo=str(data_repo))
Expand All @@ -236,14 +234,12 @@ def test_run_starts_http_and_runs_tunnel_without_doctor_or_autosync_by_default(t
with patch("fava_trails.tunnel_cli._start_http_runtime", return_value=http_process) as start_http:
with patch("fava_trails.tunnel_cli._wait_for_health") as wait_health:
with patch("fava_trails.tunnel_cli._sync_data_repo", return_value={"status": "ok"}) as sync:
with patch("subprocess.run") as run:
with patch("fava_trails.tunnel_cli._start_tunnel_client", return_value=tunnel_process) as start_tunnel:
rc = cmd_run(args)
with patch("fava_trails.tunnel_cli._start_tunnel_client", return_value=tunnel_process) as start_tunnel:
rc = cmd_run(args)

assert rc == 0
start_http.assert_called_once()
wait_health.assert_called_once()
run.assert_not_called()
start_tunnel.assert_called_once()
sync.assert_not_called()

Expand Down Expand Up @@ -363,33 +359,6 @@ def test_run_does_not_expose_when_startup_sync_result_is_unstructured(tmp_path,
start_tunnel.assert_not_called()


def test_run_checks_doctor_when_requested(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
args = _args(data_repo=str(data_repo), tunnel_doctor=True)

http_process = MagicMock()
http_process.poll.return_value = None
http_process.pid = 111
tunnel_process = MagicMock()
tunnel_process.wait.return_value = 0
tunnel_process.poll.return_value = 0
tunnel_process.pid = 222

with patch("fava_trails.tunnel_cli._find_jj_bin", return_value="/usr/bin/jj"):
with patch("shutil.which", return_value="/usr/bin/tunnel-client"):
with patch("fava_trails.tunnel_cli._check_port_available"):
with patch("fava_trails.tunnel_cli._start_http_runtime", return_value=http_process):
with patch("fava_trails.tunnel_cli._wait_for_health"):
with patch("fava_trails.tunnel_cli._sync_data_repo", return_value={"status": "ok"}):
with patch("subprocess.run", return_value=MagicMock(returncode=0)) as run:
with patch("fava_trails.tunnel_cli._start_tunnel_client", return_value=tunnel_process):
rc = cmd_run(args)

assert rc == 0
run.assert_called_once()


def test_start_writes_state_and_pid(tmp_path, monkeypatch, capsys):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
Expand Down Expand Up @@ -423,33 +392,6 @@ def fake_popen(*args, **kwargs):
assert "http://127.0.0.1:8765/mcp/" in (state_dirs[0] / "metadata.json").read_text()


def test_start_passes_tunnel_doctor_to_supervisor_when_requested(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path / "state"))
args = _args(data_repo=str(data_repo), tunnel_doctor=True)

process = MagicMock()
process.pid = 4321
process.poll.return_value = None

state_dir = tunnel_cli._state_dir(data_repo.resolve(), DEFAULT_PROFILE)

def fake_popen(command, *args, **kwargs):
state_dir.mkdir(parents=True, exist_ok=True)
tunnel_cli._ready_file(state_dir).write_text('{"status":"ready"}\n')
assert "--tunnel-doctor" in command
return process

with patch("fava_trails.tunnel_cli._find_jj_bin", return_value="/usr/bin/jj"):
with patch("shutil.which", return_value="/usr/bin/tunnel-client"):
with patch("fava_trails.tunnel_cli._check_port_available"):
with patch("subprocess.Popen", side_effect=fake_popen):
rc = cmd_start(args)

assert rc == 0


def test_start_forwards_sync_on_start_to_supervisor(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
Expand Down Expand Up @@ -752,50 +694,38 @@ def test_tunnel_cli_help_mentions_start():
assert "preflight" in help_text


def test_preflight_starts_only_private_runtime_then_cleans_up(tmp_path, monkeypatch):
@pytest.mark.parametrize(
("command", "flag"),
[
("run", "--tunnel-doctor"),
("start", "--tunnel-doctor"),
("preflight", "--tunnel-doctor"),
("preflight", "--tunnel-client"),
],
)
def test_tunnel_cli_rejects_removed_tunnel_doctor_flags(command, flag):
with pytest.raises(SystemExit):
tunnel_cli.build_parser().parse_args([command, flag])


def test_preflight_starts_only_private_runtime_without_tunnel_client(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
args = _args(data_repo=str(data_repo), tunnel_doctor=True)
args = _args(data_repo=str(data_repo))
http_process = MagicMock(pid=111)
http_process.poll.return_value = None

with patch("fava_trails.tunnel_cli._find_jj_bin", return_value="/usr/bin/jj"):
with patch("shutil.which", return_value="/usr/bin/tunnel-client"):
with patch("shutil.which", return_value=None):
with patch("fava_trails.tunnel_cli._check_port_available"):
with patch("fava_trails.tunnel_cli._start_http_runtime", return_value=http_process) as start_http:
with patch("fava_trails.tunnel_cli._wait_for_health") as wait_health:
with patch("fava_trails.tunnel_cli._run_tunnel_doctor") as doctor:
with patch("fava_trails.tunnel_cli._start_tunnel_client") as start_tunnel:
with patch("fava_trails.tunnel_cli._terminate_process") as terminate:
assert cmd_preflight(args) == 0
with patch("fava_trails.tunnel_cli._start_tunnel_client") as start_tunnel:
with patch("fava_trails.tunnel_cli._terminate_process") as terminate:
assert cmd_preflight(args) == 0

start_http.assert_called_once()
wait_health.assert_called_once()
doctor.assert_called_once()
start_tunnel.assert_not_called()
terminate.assert_called_once_with(http_process)


def test_preflight_cleans_private_runtime_when_doctor_fails(tmp_path, monkeypatch):
data_repo = _make_data_repo(tmp_path)
monkeypatch.setenv("OPENROUTER_API_KEY", "test-key")
args = _args(data_repo=str(data_repo), tunnel_doctor=True)
http_process = MagicMock(pid=111)
http_process.poll.return_value = None

with patch("fava_trails.tunnel_cli._find_jj_bin", return_value="/usr/bin/jj"):
with patch("shutil.which", return_value="/usr/bin/tunnel-client"):
with patch("fava_trails.tunnel_cli._check_port_available"):
with patch("fava_trails.tunnel_cli._start_http_runtime", return_value=http_process):
with patch("fava_trails.tunnel_cli._wait_for_health"):
with patch(
"fava_trails.tunnel_cli._run_tunnel_doctor",
side_effect=subprocess.SubprocessError("doctor failed"),
):
with patch("fava_trails.tunnel_cli._start_tunnel_client") as start_tunnel:
with patch("fava_trails.tunnel_cli._terminate_process") as terminate:
assert cmd_preflight(args) == 1

start_tunnel.assert_not_called()
terminate.assert_called_once_with(http_process)

Expand Down
Loading