From 30b6698461fc40d916f4abfc7010b65e8b8d28c8 Mon Sep 17 00:00:00 2001 From: George Finklang Date: Thu, 16 Jul 2026 13:53:53 -0700 Subject: [PATCH] feat(cli): add --port-base to shift port-probe range Adds three layers of override for the port probe's start port so users can dodge collisions with other local dev servers (Apollo BE on 8090, etc.) without editing source: 1. --port-base PORT CLI arg (per-invocation) 2. CODE_PUPPY_PORT_BASE env var (per-shell) 3. port_base key in puppy.cfg (persistent, seeded on first run) Precedence: CLI > env > puppy.cfg > 8090 default. All three sources funnel through resolve_port_base() + _coerce_port_base() for DRY validation with a consistent warning UX. Robustness: * CLI arg is type=str so bad input is caught by our validator and falls through to the next source rather than argparse-exiting. * MIN_PORT_BASE=1024 / MAX_PORT_BASE=65535-PORT_PROBE_WIDTH reject values that would probe outside the 16-bit port range or hit privileged ports the process can't bind anyway. * ensure_config_exists() seeds port_base in fresh puppy.cfg so users discover the knob (matches the existing auto_save_session seeding pattern). Tests: 13 new tests across TestGetPortBase, TestResolvePortBase, and TestPortAvailability covering precedence, bounds, graceful fallback, empty-string handling, exact boundaries, config seeding, and a regression guard on MAX_PORT_BASE + PORT_PROBE_WIDTH <= 65535. --- code_puppy/cli_runner.py | 21 ++++++- code_puppy/config.py | 70 ++++++++++++++++++++++ tests/test_cli_runner.py | 44 ++++++++++++++ tests/test_config.py | 122 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 255 insertions(+), 2 deletions(-) diff --git a/code_puppy/cli_runner.py b/code_puppy/cli_runner.py index ce38fc1fc..a75fb735d 100644 --- a/code_puppy/cli_runner.py +++ b/code_puppy/cli_runner.py @@ -198,6 +198,19 @@ async def main(): "directory; scopes to git root + branch when available)" ), ) + parser.add_argument( + "--port-base", + type=str, + default=None, + metavar="PORT", + help=( + "Starting port for the local HTTP server (searches PORT..PORT+920). " + "Bump this if 8090 collides with another local dev server. " + "Falls back to $CODE_PUPPY_PORT_BASE or 'port_base' in puppy.cfg (default 8090). " + "Invalid values are warned about and ignored -- next source in the " + "precedence chain is used instead of crashing." + ), + ) parser.add_argument( "command", nargs="*", help="Run a single command (deprecated, use -p instead)" ) @@ -272,9 +285,13 @@ async def main(): # Truecolor warning moved to interactive_mode() so it prints LAST # after all the help stuff - max visibility for the ugly red box! - available_port = find_available_port() + from code_puppy.config import PORT_PROBE_WIDTH, resolve_port_base + + port_base = resolve_port_base(cli_value=args.port_base) + port_end = port_base + PORT_PROBE_WIDTH + available_port = find_available_port(start_port=port_base, end_port=port_end) if available_port is None: - emit_error("No available ports in range 8090-9010!") + emit_error(f"No available ports in range {port_base}-{port_end}!") return # Early model setting if specified via command line diff --git a/code_puppy/config.py b/code_puppy/config.py index a79badbaf..137b04c71 100644 --- a/code_puppy/config.py +++ b/code_puppy/config.py @@ -302,6 +302,10 @@ def ensure_config_exists(): # Set default values for important config keys if they don't exist if not config[DEFAULT_SECTION].get("auto_save_session"): config[DEFAULT_SECTION]["auto_save_session"] = "true" + # port_base: seed so users discover the knob in their generated puppy.cfg + # (starting port for the HTTP-server port probe; searches port_base..+920). + if not config[DEFAULT_SECTION].get("port_base"): + config[DEFAULT_SECTION]["port_base"] = str(DEFAULT_PORT_BASE) # Write the config if we made any changes if missing or not exists: @@ -2841,3 +2845,69 @@ def get_frontend_emitter_queue_size() -> int: return int(val) except ValueError: return 100 + + +# Port-probe bounds: +# MIN_PORT_BASE=1024 avoids privileged ports the user process can't bind anyway. +# PORT_PROBE_WIDTH is how many consecutive ports find_available_port() scans. +# MAX_PORT_BASE keeps port_base + width within the 16-bit port space. +MIN_PORT_BASE = 1024 +PORT_PROBE_WIDTH = 920 +MAX_PORT_BASE = 65535 - PORT_PROBE_WIDTH +DEFAULT_PORT_BASE = 8090 + + +def _coerce_port_base(raw, source: str) -> int | None: + """Parse + range-check a candidate port_base. Returns None (with warning) + on invalid input so callers can fall through to the next source. + """ + if raw is None or (isinstance(raw, str) and not raw.strip()): + return None + try: + val = int(str(raw).strip()) + except (TypeError, ValueError): + _warn_port_base(f"Ignoring invalid {source} port_base={raw!r}: not an integer") + return None + if not (MIN_PORT_BASE <= val <= MAX_PORT_BASE): + _warn_port_base( + f"Ignoring {source} port_base={val}: must be in " + f"[{MIN_PORT_BASE}, {MAX_PORT_BASE}] so port+{PORT_PROBE_WIDTH} stays valid" + ) + return None + return val + + +def _warn_port_base(msg: str) -> None: + """Lazy-import emit_warning to avoid config <-> messaging import cycles.""" + try: + from code_puppy.messaging import emit_warning + + emit_warning(msg) + except Exception: + # Messaging bus not up yet (early startup); silent skip is fine -- + # the fallback value still applies. + pass + + +def resolve_port_base(cli_value=None) -> int: + """ + Full precedence chain for the port probe's starting port: + CLI --port-base > CODE_PUPPY_PORT_BASE env > puppy.cfg[port_base] > default. + + Invalid values at any layer are warned about and skipped, not crashed on. + """ + candidates = ( + (cli_value, "--port-base"), + (os.environ.get("CODE_PUPPY_PORT_BASE"), "CODE_PUPPY_PORT_BASE"), + (get_value("port_base"), "puppy.cfg[port_base]"), + ) + for raw, source in candidates: + val = _coerce_port_base(raw, source) + if val is not None: + return val + return DEFAULT_PORT_BASE + + +def get_port_base() -> int: + """Back-compat wrapper: resolve without a CLI-supplied value.""" + return resolve_port_base(cli_value=None) diff --git a/tests/test_cli_runner.py b/tests/test_cli_runner.py index 19e992b6d..1fc20ac28 100644 --- a/tests/test_cli_runner.py +++ b/tests/test_cli_runner.py @@ -338,6 +338,50 @@ def test_find_available_port_called(self, mock_find_port): result = find_available_port() assert result is not None + def test_port_base_argparse_default_is_none(self): + """--port-base defaults to None so cli_runner can fall back to config. + + If this ever flips to a hardcoded value, the env-var / puppy.cfg + precedence chain silently breaks. + """ + import argparse + + parser = argparse.ArgumentParser() + # Mirror production: type=str (not int) so bad input is validated + # gracefully in resolve_port_base rather than argparse-exiting. + parser.add_argument("--port-base", type=str, default=None) + args = parser.parse_args([]) + assert args.port_base is None + args = parser.parse_args(["--port-base", "9100"]) + assert args.port_base == "9100" + + def test_port_base_cli_wins_over_config(self): + """resolve_port_base must honor a valid CLI value over env/cfg.""" + from code_puppy.config import resolve_port_base + + with patch("code_puppy.config.get_value", return_value="9500"): + with patch.dict("os.environ", {"CODE_PUPPY_PORT_BASE": "9700"}): + assert resolve_port_base(cli_value="9100") == 9100 + assert resolve_port_base(cli_value=None) == 9700 # falls to env + + def test_bad_cli_port_base_does_not_crash(self): + """Garbage --port-base must be skipped, not raise SystemExit. + + This is the whole point of type=str + resolve_port_base -- + argparse type=int would hard-exit before we could recover. + """ + from code_puppy.config import DEFAULT_PORT_BASE, resolve_port_base + + with patch("code_puppy.config.get_value", return_value=None): + with patch.dict("os.environ", {}, clear=False): + import os + + os.environ.pop("CODE_PUPPY_PORT_BASE", None) + assert ( + resolve_port_base(cli_value="totally-not-a-port") + == DEFAULT_PORT_BASE + ) + class TestAgentRunning: """Test agent execution.""" diff --git a/tests/test_config.py b/tests/test_config.py index 2b3d2d229..b1d55c9d9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1031,3 +1031,125 @@ def test_rejects_path_traversal(self): def test_rejects_whitespace_and_control(self): assert cp_config._is_valid_autosave_session_name("bad name") is False assert cp_config._is_valid_autosave_session_name("") is False + + +class TestGetPortBase: + """Precedence for get_port_base: env var > puppy.cfg > default. + + Also covers bounds validation and graceful skipping of invalid sources. + """ + + @patch("code_puppy.config.get_value") + def test_env_var_overrides_cfg(self, mock_get_value, monkeypatch): + mock_get_value.return_value = "9500" + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "9700") + assert cp_config.get_port_base() == 9700 + + @patch("code_puppy.config.get_value") + def test_cfg_used_when_no_env(self, mock_get_value, monkeypatch): + mock_get_value.return_value = "9500" + monkeypatch.delenv("CODE_PUPPY_PORT_BASE", raising=False) + assert cp_config.get_port_base() == 9500 + + @patch("code_puppy.config.get_value") + def test_default_when_nothing_set(self, mock_get_value, monkeypatch): + mock_get_value.return_value = None + monkeypatch.delenv("CODE_PUPPY_PORT_BASE", raising=False) + assert cp_config.get_port_base() == cp_config.DEFAULT_PORT_BASE + + @patch("code_puppy.config.get_value") + def test_bad_env_value_skips_to_cfg(self, mock_get_value, monkeypatch): + # env is garbage -> should fall through to cfg, not crash + mock_get_value.return_value = "9200" + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "not-a-number") + assert cp_config.get_port_base() == 9200 + + @patch("code_puppy.config.get_value") + def test_bad_env_and_bad_cfg_uses_default(self, mock_get_value, monkeypatch): + mock_get_value.return_value = "also-not-a-number" + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "not-a-number") + assert cp_config.get_port_base() == cp_config.DEFAULT_PORT_BASE + + @patch("code_puppy.config.get_value") + def test_whitespace_stripped(self, mock_get_value, monkeypatch): + mock_get_value.return_value = None + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", " 9300 ") + assert cp_config.get_port_base() == 9300 + + @patch("code_puppy.config.get_value") + def test_empty_string_skipped(self, mock_get_value, monkeypatch): + # Empty env string shouldn't shadow puppy.cfg. + mock_get_value.return_value = "9400" + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "") + assert cp_config.get_port_base() == 9400 + + @patch("code_puppy.config.get_value") + def test_below_min_port_base_rejected(self, mock_get_value, monkeypatch): + # Privileged port (< 1024) -> skip and fall through. + mock_get_value.return_value = None + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "80") + assert cp_config.get_port_base() == cp_config.DEFAULT_PORT_BASE + + @patch("code_puppy.config.get_value") + def test_above_max_port_base_rejected(self, mock_get_value, monkeypatch): + # port_base + PORT_PROBE_WIDTH would exceed 65535 -> skip. + mock_get_value.return_value = None + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", str(cp_config.MAX_PORT_BASE + 1)) + assert cp_config.get_port_base() == cp_config.DEFAULT_PORT_BASE + + @patch("code_puppy.config.get_value") + def test_exact_boundaries_accepted(self, mock_get_value, monkeypatch): + mock_get_value.return_value = None + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", str(cp_config.MIN_PORT_BASE)) + assert cp_config.get_port_base() == cp_config.MIN_PORT_BASE + + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", str(cp_config.MAX_PORT_BASE)) + assert cp_config.get_port_base() == cp_config.MAX_PORT_BASE + + def test_probe_width_keeps_top_port_valid(self): + # Regression guard: MAX_PORT_BASE + PORT_PROBE_WIDTH must fit in a + # 16-bit port. If someone bumps PORT_PROBE_WIDTH without adjusting + # MAX_PORT_BASE this test will fail loudly. + assert cp_config.MAX_PORT_BASE + cp_config.PORT_PROBE_WIDTH <= 65535 + + def test_ensure_config_exists_seeds_port_base(self, mock_config_paths, monkeypatch): + """Fresh puppy.cfg should include port_base so users discover the knob.""" + _, mock_cfg_file = mock_config_paths + monkeypatch.setattr(os.path, "exists", MagicMock(return_value=False)) + monkeypatch.setattr(os.path, "isfile", MagicMock(return_value=False)) + monkeypatch.setattr(os, "makedirs", MagicMock()) + monkeypatch.setattr( + "builtins.input", + MagicMock(side_effect=lambda _: "stub"), + ) + + with patch("builtins.open", mock_open()): + config_parser = cp_config.ensure_config_exists() + + assert config_parser.get(DEFAULT_SECTION_NAME, "port_base") == str( + cp_config.DEFAULT_PORT_BASE + ) + + +class TestResolvePortBase: + """CLI value takes highest priority, invalid CLI value falls through.""" + + @patch("code_puppy.config.get_value") + def test_cli_value_wins(self, mock_get_value, monkeypatch): + mock_get_value.return_value = "9500" + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "9700") + assert cp_config.resolve_port_base(cli_value="9100") == 9100 + assert cp_config.resolve_port_base(cli_value=9100) == 9100 + + @patch("code_puppy.config.get_value") + def test_bad_cli_falls_through_to_env(self, mock_get_value, monkeypatch): + mock_get_value.return_value = None + monkeypatch.setenv("CODE_PUPPY_PORT_BASE", "9700") + # Non-integer CLI input must NOT crash -- next source wins. + assert cp_config.resolve_port_base(cli_value="garbage") == 9700 + + @patch("code_puppy.config.get_value") + def test_none_cli_defers_to_lower_layers(self, mock_get_value, monkeypatch): + mock_get_value.return_value = "9500" + monkeypatch.delenv("CODE_PUPPY_PORT_BASE", raising=False) + assert cp_config.resolve_port_base(cli_value=None) == 9500