Skip to content

Commit cd08aaf

Browse files
authored
fix: restore --proxy for base_url (#9503)
## 📝 Summary closes #9495 by restoring `-proxy` behavior to set `--base_path` if the full path is provided.
1 parent 0eecccd commit cd08aaf

3 files changed

Lines changed: 175 additions & 5 deletions

File tree

marimo/_cli/cli.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,8 @@ def edit(
583583
# Check for version updates after preflight checks pass.
584584
check_for_updates(print_latest_version)
585585

586+
base_url = validators.check_proxy_base_url(proxy, base_url)
587+
586588
start(
587589
workspace=infer_workspace(name),
588590
development_mode=GLOBAL_SETTINGS.DEVELOPMENT_MODE,
@@ -790,6 +792,8 @@ def _cleanup() -> None:
790792
if workspace is None:
791793
workspace = EmptyWorkspace()
792794

795+
base_url = validators.check_proxy_base_url(proxy, base_url)
796+
793797
start(
794798
workspace=workspace,
795799
development_mode=GLOBAL_SETTINGS.DEVELOPMENT_MODE,
@@ -1212,6 +1216,8 @@ def run(
12121216

12131217
workspace = _create_run_workspace(validated_paths, watch=watch)
12141218

1219+
base_url = validators.check_proxy_base_url(proxy, base_url)
1220+
12151221
start(
12161222
workspace=workspace,
12171223
development_mode=GLOBAL_SETTINGS.DEVELOPMENT_MODE,
@@ -1341,6 +1347,8 @@ def tutorial(
13411347
temp_dir = tempfile.TemporaryDirectory()
13421348
path = create_temp_tutorial_file(name, temp_dir)
13431349

1350+
base_url = validators.check_proxy_base_url(proxy, "")
1351+
13441352
start(
13451353
workspace=SingleFileWorkspace.from_path(path),
13461354
development_mode=GLOBAL_SETTINGS.DEVELOPMENT_MODE,
@@ -1360,6 +1368,7 @@ def tutorial(
13601368
token_password=token_password,
13611369
token_password_file=token_password_file,
13621370
),
1371+
base_url=base_url,
13631372
redirect_console_to_browser=False,
13641373
ttl_seconds=None,
13651374
startup_tip=choose_startup_tip(click.get_current_context()),

marimo/_cli/cli_validators.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,36 @@
33

44
from pathlib import Path
55
from typing import Any
6+
from urllib.parse import urlparse
67

78
import click
89

910

11+
def check_proxy_base_url(proxy: str | None, base_url_value: str) -> str:
12+
"""Reconcile --proxy path with --base-url. Returns the effective
13+
base_url: explicit --base-url if given, else the path component from
14+
--proxy, else empty. Raises click.BadParameter when both are given
15+
and disagree (after trailing-slash normalization).
16+
"""
17+
if not proxy:
18+
return base_url_value
19+
parse_target = proxy if "://" in proxy else f"//{proxy}"
20+
try:
21+
proxy_path = urlparse(parse_target).path
22+
except ValueError:
23+
return base_url_value
24+
proxy_path = proxy_path.rstrip("/")
25+
if not proxy_path:
26+
return base_url_value
27+
if base_url_value and proxy_path != base_url_value.rstrip("/"):
28+
raise click.BadParameter(
29+
f"--proxy path {proxy_path!r} conflicts with --base-url "
30+
f"{base_url_value!r}; use one or make them match.",
31+
param_hint="--proxy",
32+
)
33+
return base_url_value or proxy_path
34+
35+
1036
def base_url(ctx: Any, param: Any, value: str | None) -> str:
1137
del ctx
1238
del param

tests/_server/test_startup_url.py

Lines changed: 140 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
# Copyright 2026 Marimo. All rights reserved.
22
from __future__ import annotations
33

4-
from unittest.mock import MagicMock
4+
from unittest.mock import MagicMock, patch
5+
from urllib.parse import urlparse
56

67
import pytest
8+
from click.testing import CliRunner
79

10+
from marimo._cli.cli import main as cli_main
11+
from marimo._cli.cli_validators import check_proxy_base_url
812
from marimo._server.api.lifespans import _startup_url
913
from marimo._server.start import _resolve_proxy
1014
from marimo._server.tokens import AuthToken
@@ -85,10 +89,143 @@ def test_resolve_proxy(
8589
assert host == expected_host
8690

8791

92+
def test_cli_errors_when_proxy_path_conflicts_with_base_url() -> None:
93+
"""Passing both `--proxy host/foo` and `--base-url /bar` is ambiguous."""
94+
runner = CliRunner()
95+
# Patch the symbol the edit command imported, so the test never
96+
# actually launches a server even if validation fails to fire.
97+
with patch("marimo._cli.cli.start") as mock_start:
98+
result = runner.invoke(
99+
cli_main,
100+
[
101+
"edit",
102+
"--proxy",
103+
"example.com/foo",
104+
"--base-url",
105+
"/bar",
106+
"--no-token",
107+
"--headless",
108+
"--skip-update-check",
109+
],
110+
)
111+
112+
assert result.exit_code != 0, result.output
113+
assert mock_start.call_count == 0, (
114+
"start() should not be invoked when --proxy and --base-url "
115+
f"conflict; output was: {result.output!r}"
116+
)
117+
118+
119+
def test_cli_accepts_proxy_path_matching_base_url() -> None:
120+
runner = CliRunner()
121+
with patch("marimo._cli.cli.start") as mock_start:
122+
result = runner.invoke(
123+
cli_main,
124+
[
125+
"edit",
126+
"--proxy",
127+
"example.com/foo",
128+
"--base-url",
129+
"/foo",
130+
"--no-token",
131+
"--headless",
132+
"--skip-update-check",
133+
],
134+
)
135+
136+
assert result.exit_code == 0, result.output
137+
assert mock_start.call_count == 1, (
138+
"start() should be invoked when --proxy path matches --base-url; "
139+
f"output was: {result.output!r}"
140+
)
141+
142+
143+
def test_cli_tutorial_routes_proxy_path_to_base_url() -> None:
144+
"""`tutorial` has no --base-url flag; the proxy path must still be
145+
captured (regression caught by codex review).
146+
"""
147+
runner = CliRunner()
148+
with patch("marimo._cli.cli.start") as mock_start:
149+
result = runner.invoke(
150+
cli_main,
151+
[
152+
"tutorial",
153+
"--proxy",
154+
"example.com/foo",
155+
"--no-token",
156+
"--headless",
157+
"intro",
158+
],
159+
)
160+
161+
assert result.exit_code == 0, result.output
162+
assert mock_start.call_count == 1
163+
# Proxy path should have flowed into base_url
164+
assert mock_start.call_args.kwargs["base_url"] == "/foo"
165+
166+
167+
def test_cli_accepts_proxy_without_path_and_explicit_base_url() -> None:
168+
runner = CliRunner()
169+
with patch("marimo._cli.cli.start") as mock_start:
170+
result = runner.invoke(
171+
cli_main,
172+
[
173+
"edit",
174+
"--proxy",
175+
"example.com:8080",
176+
"--base-url",
177+
"/foo",
178+
"--no-token",
179+
"--headless",
180+
"--skip-update-check",
181+
],
182+
)
183+
184+
assert result.exit_code == 0, result.output
185+
assert mock_start.call_count == 1, (
186+
"start() should be invoked when --proxy has no path; "
187+
f"output was: {result.output!r}"
188+
)
189+
190+
191+
def test_check_proxy_base_url_normalizes_trailing_slash() -> None:
192+
"""The validator strips the trailing / from the proxy path so the
193+
resulting base_url matches the --base-url validator's own contract
194+
(no trailing /).
195+
"""
196+
assert check_proxy_base_url("example.com/proxy/2718/", "") == "/proxy/2718"
197+
198+
199+
def test_startup_url_proxy_path_equivalent_to_explicit_base_url() -> None:
200+
"""`--proxy host/foo` ≡ `--proxy host --base-url=/foo` in printed URL —
201+
after the CLI validator merges the proxy path into base_url.
202+
"""
203+
# Branch A: path embedded in --proxy, no separate --base-url
204+
port_a, host_a = _resolve_proxy(
205+
2718, "127.0.0.1", "example.com/proxy/2718"
206+
)
207+
effective_a = check_proxy_base_url("example.com/proxy/2718", "")
208+
state_a = _make_state(host=host_a, port=port_a, base_url=effective_a)
209+
210+
# Branch B: same intent expressed as --proxy host + --base-url /path
211+
port_b, host_b = _resolve_proxy(2718, "127.0.0.1", "example.com")
212+
state_b = _make_state(host=host_b, port=port_b, base_url="/proxy/2718")
213+
214+
assert _startup_url(state_a) == _startup_url(state_b)
215+
216+
217+
def test_startup_url_proxy_with_non_default_port_preserves_path() -> None:
218+
"""Path lives in state.base_url (not state.host), so port + bracketing
219+
compose cleanly.
220+
"""
221+
port, host = _resolve_proxy(2718, "127.0.0.1", "example.com:8080/foo")
222+
effective = check_proxy_base_url("example.com:8080/foo", "")
223+
state = _make_state(host=host, port=port, base_url=effective)
224+
assert _startup_url(state) == "http://example.com:8080/foo"
225+
226+
88227
def test_startup_url_getnameinfo_failure() -> None:
89228
"""If getnameinfo raises (e.g. host not resolvable), URL is still valid."""
90-
from unittest.mock import patch
91-
92229
state = _make_state("fd00::dead")
93230
with patch("socket.getnameinfo", side_effect=OSError("unreachable")):
94231
url = _startup_url(state)
@@ -102,8 +239,6 @@ def test_startup_url_ipv6_with_token() -> None:
102239
url = _startup_url(state)
103240
assert url == "http://[fd00::cafe]:2718/?access_token=tok3n"
104241
# Verify it's parseable and components are correct
105-
from urllib.parse import urlparse
106-
107242
parsed = urlparse(url)
108243
assert parsed.hostname == "fd00::cafe"
109244
assert parsed.port == 2718

0 commit comments

Comments
 (0)