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