Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 19 additions & 2 deletions code_puppy/cli_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
)
Expand Down Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions code_puppy/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
44 changes: 44 additions & 0 deletions tests/test_cli_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
122 changes: 122 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading