phase: 01_runnable_service_scaffold

**Phase 01 final verification pass — all criteria green** - Verified (nothing left to implement; all 4 task files already in `complete/`): toolchain lock, typed `Settings`+`.env.example`, debugpy bootstrap, app factory, CLI (`--host/--port/--reload/--levels-dir`, `python -m undercooked`), static shell + `/static`, security headers, no-CDN/doc guards, `validate.sh`. - `uv sync --extra dev` → exit 0; `uv.lock` present (untracked → harness commits it with the phase). - `uv run undercooked --host 127.0.0.1 --port 8000` → `curl /healthz` = 200 `{"status":"ok","version":"0.1.0","rooms":0}` (superset of the criterion's body — now documented in README); `curl /` = 200 shell HTML; static css/js 200; CSP/nosniff/no-referrer/DENY on 200, 404 and 405 too. - `DEBUGPY=1`: port 5678 is held on this machine by the user's own podman dev-container debugpy adapter — **no live change made/killed**; verified via `DEBUGPY=1 DEBUGPY_PORT=5679` → listener opens, `/healthz` still 200; the occupied-port case degraded gracefully as designed (warning, server serves). Bad config → one line + exit 2 ✓. - `bash .agents/validate.sh` (root **and** `.agents/`) → exit 0: `ruff check .` 0 errors, `pyright` 0 errors, levels-CLI notice-skip (matches README prose), **284 passed** full suite incl. 10 Playwright e2e, coverage **100.00%** (floor 90), repo hygiene clean. `uv run pytest -m e2e` → 10 passed. - Docs fixed: README Status → phase 01 complete + next pending 02 (was pointing at `todo/01…`), documented real `/healthz` body, precedence now "CLI flags > env > .env > default", added the PLAN §13-promised **How to play** section (honest about shell-only today), Layout rows marked what exists vs phases 02/07. AGENTS.md: recorded the `data:`-URL exemption in the no-CDN rule (test allows it) and matched the session-token rule to the browser's WebCrypto/base64url implementation. Also corrected a stale "session-wide chromium" docstring in `tests/e2e/conftest.py` (fixtures are function-scoped). - Deviations: no container build run (needs an image pull; not a phase criterion — `Containerfile`/`compose.yaml` reviewed and consistent with `docs/DEPLOYMENT.md`); `HEAD /` is 405 (routes are GET-only by design; health checks use GET). No git index/history commands run. - Next pending phase: `02_level_content_and_validation`.
2026-10-06 01:51:20 -04:00
parent f84c6ebaef
commit 6fd83446ec
43 changed files with 5478 additions and 30 deletions
@@ -0,0 +1,471 @@
+"""Unit tests for `undercooked.config` (PLAN.md A17): defaults, overrides, validation.
+
+Everything here calls `load_settings({...})` with an explicit mapping wherever
+possible, so a developer's local `.env` can never change what a test asserts.
+The `.env` / cache behaviour is exercised separately, in an isolated directory.
+"""
+
+from __future__ import annotations
+
+import dataclasses
+import os
+import subprocess
+import sys
+from collections.abc import Callable, Iterator
+from pathlib import Path
+
+import pytest
+
+from undercooked import config
+from undercooked.config import (
+    ENV_KEYS,
+    ConfigError,
+    Settings,
+    apply_cli_overrides,
+    get_settings,
+    load_settings,
+    reset_settings_cache,
+)
+
+pytestmark = pytest.mark.unit
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+
+
+@pytest.fixture(autouse=True)
+def pristine_env() -> Iterator[None]:
+    """Undo env/cache side effects - `load_dotenv` mutates `os.environ` in place."""
+    saved = dict(os.environ)
+    reset_settings_cache()
+    yield
+    os.environ.clear()
+    os.environ.update(saved)
+    reset_settings_cache()
+
+
+# ----------------------------------------------------------------------------- defaults
+
+
+def test_defaults_match_the_documented_table() -> None:
+    settings = load_settings({})
+    assert settings == Settings()
+    assert settings.host == "127.0.0.1"
+    assert settings.port == 8000
+    assert settings.tick_hz == 20
+    assert settings.snapshot_hz == 20
+    assert settings.max_players == 4
+    assert settings.max_rooms == 200
+    assert settings.levels_dir == Path("levels")
+    assert settings.static_dir is None  # derived from the package layout, see server.app
+    assert settings.lobby_idle_timeout_s == 900.0
+    assert settings.guest_idle_timeout_s == 45.0
+    assert settings.max_message_bytes == 4096
+    assert settings.max_inputs_per_s == 40
+    assert settings.sim_latency_ms == 0
+    assert settings.debugpy is False
+    assert settings.debugpy_port == 5678
+    assert settings.allowed_origins == ()
+
+
+def test_env_keys_cover_exactly_the_settings_fields() -> None:
+    assert set(ENV_KEYS) == {field.name for field in dataclasses.fields(Settings)}
+
+
+def test_settings_is_frozen_and_uses_slots() -> None:
+    settings = Settings()
+    with pytest.raises(dataclasses.FrozenInstanceError):
+        settings.port = 1234  # type: ignore[misc]
+    assert not hasattr(settings, "__dict__")
+
+
+def test_config_error_is_a_value_error() -> None:
+    assert issubclass(ConfigError, ValueError)
+
+
+# ----------------------------------------------------------------------------- overrides
+
+
+@pytest.mark.parametrize(
+    ("field", "raw", "expected"),
+    [
+        ("host", "localhost", "localhost"),
+        ("port", "8123", 8123),
+        ("tick_hz", "30", 30),
+        ("snapshot_hz", "15", 15),
+        ("max_players", "2", 2),
+        ("max_rooms", "1", 1),
+        ("lobby_idle_timeout_s", "60", 60.0),
+        ("guest_idle_timeout_s", "7.5", 7.5),
+        ("max_message_bytes", "8192", 8192),
+        ("max_inputs_per_s", "60", 60),
+        ("sim_latency_ms", "150", 150),
+        ("debugpy_port", "5679", 5679),
+    ],
+)
+def test_single_variable_override(field: str, raw: str, expected: object) -> None:
+    settings = load_settings({ENV_KEYS[field]: raw})
+    assert getattr(settings, field) == expected
+    assert settings.levels_dir == Path("levels")  # untouched fields keep their default
+
+
+@pytest.mark.parametrize("raw", ["1", "true", "TRUE", "yes", "on"])
+def test_debugpy_truthy_values(raw: str) -> None:
+    assert load_settings({"DEBUGPY": raw}).debugpy is True
+
+
+@pytest.mark.parametrize("raw", ["0", "false", "False", "no", "off"])
+def test_debugpy_falsy_values(raw: str) -> None:
+    assert load_settings({"DEBUGPY": raw}).debugpy is False
+
+
+@pytest.mark.parametrize(
+    ("raw", "expected"),
+    [
+        ("", ()),
+        ("   ", ()),
+        ("http://localhost:3000", ("http://localhost:3000",)),
+        ("http://a , http://b", ("http://a", "http://b")),
+        ("http://a,,http://b,", ("http://a", "http://b")),
+        (",,", ()),
+    ],
+)
+def test_allowed_origins_is_comma_split(raw: str, expected: tuple[str, ...]) -> None:
+    assert load_settings({ENV_KEYS["allowed_origins"]: raw}).allowed_origins == expected
+
+
+@pytest.mark.parametrize(
+    ("raw", "expected"),
+    [
+        ("levels", Path("levels")),
+        ("levels/", Path("levels")),
+        ("/srv/undercooked/levels", Path("/srv/undercooked/levels")),
+        ("~/kitchens", Path.home() / "kitchens"),
+        ("./kitchens", Path("kitchens")),
+    ],
+)
+def test_levels_dir_path_handling(raw: str, expected: Path) -> None:
+    assert load_settings({ENV_KEYS["levels_dir"]: raw}).levels_dir == expected
+
+
+@pytest.mark.parametrize(
+    ("raw", "expected"),
+    [
+        ("/app/app/static", Path("/app/app/static")),
+        ("~/static", Path.home() / "static"),
+        ("static/", Path("static")),
+        ("", None),
+        ("   ", None),
+    ],
+)
+def test_static_dir_is_optional_and_expanded(raw: str, expected: Path | None) -> None:
+    """An unset `UNDERCOOKED_STATIC_DIR` means "derive it from the package"."""
+    assert load_settings({ENV_KEYS["static_dir"]: raw}).static_dir == expected
+
+
+@pytest.mark.parametrize(
+    "field", ["port", "tick_hz", "lobby_idle_timeout_s", "host", "allowed_origins"]
+)
+@pytest.mark.parametrize("blank", ["", "   ", "\t"])
+def test_blank_value_counts_as_unset(field: str, blank: str) -> None:
+    assert load_settings({ENV_KEYS[field]: blank}) == Settings()
+
+
+def test_surrounding_whitespace_is_tolerated() -> None:
+    settings = load_settings({ENV_KEYS["port"]: " 8123 ", ENV_KEYS["host"]: "\nlocalhost\t"})
+    assert (settings.port, settings.host) == (8123, "localhost")
+
+
+def test_snapshot_hz_may_equal_tick_hz_but_not_exceed_it() -> None:
+    both_ten = load_settings({ENV_KEYS["tick_hz"]: "10", ENV_KEYS["snapshot_hz"]: "10"})
+    assert both_ten.snapshot_hz == 10
+    with pytest.raises(ConfigError, match=r"UNDERCOOKED_SNAPSHOT_HZ.*must be >= 1, <= 10"):
+        load_settings({ENV_KEYS["tick_hz"]: "10", ENV_KEYS["snapshot_hz"]: "20"})
+
+
+# ----------------------------------------------------------------------------- validation errors
+
+
+@pytest.mark.parametrize(
+    ("field", "raw", "message"),
+    [
+        ("port", "http", "UNDERCOOKED_PORT='http' is not an integer"),
+        ("port", "8.5", "UNDERCOOKED_PORT='8.5' is not an integer"),
+        ("port", "0", "UNDERCOOKED_PORT=0 is invalid: must be >= 1, <= 65535"),
+        ("port", "65536", "UNDERCOOKED_PORT=65536 is invalid: must be >= 1, <= 65535"),
+        ("tick_hz", "4", "UNDERCOOKED_TICK_HZ=4 is invalid: must be >= 5, <= 60"),
+        ("tick_hz", "99", "UNDERCOOKED_TICK_HZ=99 is invalid: must be >= 5, <= 60"),
+        ("tick_hz", "twenty", "UNDERCOOKED_TICK_HZ='twenty' is not an integer"),
+        ("snapshot_hz", "0", "UNDERCOOKED_SNAPSHOT_HZ=0 is invalid: must be >= 1, <= 20"),
+        ("max_players", "0", "UNDERCOOKED_MAX_PLAYERS=0 is invalid: must be >= 1, <= 4"),
+        ("max_players", "5", "UNDERCOOKED_MAX_PLAYERS=5 is invalid: must be >= 1, <= 4"),
+        ("max_rooms", "0", "UNDERCOOKED_MAX_ROOMS=0 is invalid: must be >= 1"),
+        ("max_rooms", "-3", "UNDERCOOKED_MAX_ROOMS=-3 is invalid: must be >= 1"),
+        (
+            "lobby_idle_timeout_s",
+            "-1",
+            "UNDERCOOKED_LOBBY_IDLE_TIMEOUT_S=-1.0 is invalid: must be >= 0.0",
+        ),
+        ("lobby_idle_timeout_s", "soon", "UNDERCOOKED_LOBBY_IDLE_TIMEOUT_S='soon' is not a number"),
+        (
+            "guest_idle_timeout_s",
+            "-0.5",
+            "UNDERCOOKED_GUEST_IDLE_TIMEOUT_S=-0.5 is invalid: must be >= 0.0",
+        ),
+        (
+            "max_message_bytes",
+            "255",
+            "UNDERCOOKED_MAX_MESSAGE_BYTES=255 is invalid: must be >= 256",
+        ),
+        ("max_inputs_per_s", "0", "UNDERCOOKED_MAX_INPUTS_PER_S=0 is invalid: must be >= 1"),
+        ("sim_latency_ms", "-1", "UNDERCOOKED_SIM_LATENCY_MS=-1 is invalid: must be >= 0"),
+        ("debugpy_port", "0", "DEBUGPY_PORT=0 is invalid: must be >= 1, <= 65535"),
+        (
+            "debugpy",
+            "maybe",
+            "DEBUGPY='maybe' is not a boolean (use 1/0, true/false, yes/no, on/off)",
+        ),
+    ],
+)
+def test_invalid_values_raise_config_error(field: str, raw: str, message: str) -> None:
+    with pytest.raises(ConfigError) as excinfo:
+        load_settings({ENV_KEYS[field]: raw})
+    assert str(excinfo.value) == message
+
+
+def test_out_of_range_tick_hz_fails_from_the_real_environment() -> None:
+    """The documented criterion: `UNDERCOOKED_TICK_HZ=99` fails loudly at startup."""
+    env = {
+        key: value
+        for key, value in os.environ.items()
+        if not key.startswith(("UNDERCOOKED_", "DEBUGPY"))
+    }
+    env["UNDERCOOKED_TICK_HZ"] = "99"
+    result = subprocess.run(  # fixed argv, repo's own interpreter
+        [sys.executable, "-c", "from undercooked.config import load_settings; load_settings()"],
+        cwd=REPO_ROOT,
+        env=env,
+        capture_output=True,
+        text=True,
+        check=False,
+        timeout=120,
+    )
+    assert result.returncode != 0
+    assert "ConfigError" in result.stderr
+    assert "UNDERCOOKED_TICK_HZ=99" in result.stderr
+
+
+# ----------------------------------------------------------------------------- cached accessor
+
+
+def test_get_settings_caches_until_reset(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    monkeypatch.chdir(tmp_path)
+    monkeypatch.setenv("UNDERCOOKED_PORT", "8123")
+    first = get_settings()
+    assert first.port == 8123
+    monkeypatch.setenv("UNDERCOOKED_PORT", "9999")
+    assert get_settings() is first  # still the cached object
+    reset_settings_cache()
+    assert get_settings().port == 9999
+
+
+def test_get_settings_applies_defaults_with_an_empty_environment(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    for key in list(os.environ):
+        if key.startswith(("UNDERCOOKED_", "DEBUGPY")):
+            monkeypatch.delenv(key, raising=False)
+    monkeypatch.setenv("PYTHON_DOTENV_DISABLED", "true")
+    assert get_settings() == Settings()
+
+
+# ----------------------------------------------------------------------------- .env loading
+
+
+def test_dotenv_file_is_read_from_the_current_directory(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    monkeypatch.delenv("UNDERCOOKED_PORT", raising=False)
+    monkeypatch.delenv("UNDERCOOKED_MAX_ROOMS", raising=False)
+    (tmp_path / ".env").write_text(
+        "UNDERCOOKED_PORT=8123\nUNDERCOOKED_MAX_ROOMS=7\n", encoding="utf-8"
+    )
+    monkeypatch.chdir(tmp_path)
+    settings = load_settings()
+    assert (settings.port, settings.max_rooms) == (8123, 7)
+
+
+def test_dotenv_is_found_in_a_parent_directory(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    monkeypatch.delenv("UNDERCOOKED_PORT", raising=False)
+    (tmp_path / ".env").write_text("UNDERCOOKED_PORT=8245\n", encoding="utf-8")
+    nested = tmp_path / "src" / "undercooked"
+    nested.mkdir(parents=True)
+    monkeypatch.chdir(nested)
+    assert load_settings().port == 8245
+
+
+def test_dotenv_never_overrides_an_exported_variable(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    monkeypatch.setenv("UNDERCOOKED_PORT", "7000")
+    (tmp_path / ".env").write_text("UNDERCOOKED_PORT=8123\n", encoding="utf-8")
+    monkeypatch.chdir(tmp_path)
+    assert load_settings().port == 7000
+
+
+def test_dotenv_is_read_once_until_the_cache_is_reset(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    loaded: list[str] = []
+    (tmp_path / ".env").write_text("UNDERCOOKED_PORT=8123\n", encoding="utf-8")
+    monkeypatch.chdir(tmp_path)
+    def fake_load_dotenv(path: object, **_kwargs: object) -> bool:
+        loaded.append(str(path))
+        return True
+
+    monkeypatch.setattr(config, "load_dotenv", fake_load_dotenv)
+    load_settings()
+    load_settings()
+    assert len(loaded) == 1  # one `.env` read per process, not one per call
+    reset_settings_cache()
+    load_settings()
+    assert len(loaded) == 2
+
+
+def test_missing_dotenv_file_is_not_an_error(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    monkeypatch.delenv("UNDERCOOKED_PORT", raising=False)
+    monkeypatch.chdir(tmp_path)
+    assert load_settings().port == 8000
+
+
+def test_explicit_env_mapping_skips_dotenv(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    monkeypatch.chdir(tmp_path)
+    def never_find(**_kwargs: object) -> str:
+        pytest.fail("dotenv must not be read when an explicit env mapping is given")
+
+    monkeypatch.setattr(config, "find_dotenv", never_find)
+    assert load_settings({"UNDERCOOKED_PORT": "8123"}).port == 8123
+
+
+# ----------------------------------------------------------------------------- .env.example
+
+
+def parse_env_example(path: Path) -> dict[str, str]:
+    """Read `KEY=value` lines from an env template, ignoring comments."""
+    values: dict[str, str] = {}
+    for line in path.read_text(encoding="utf-8").splitlines():
+        stripped = line.strip()
+        if not stripped or stripped.startswith("#") or "=" not in stripped:
+            continue
+        key, _, value = stripped.partition("=")
+        values[key.strip()] = value.strip()
+    return values
+
+
+def test_env_example_documents_every_variable_with_its_default() -> None:
+    example = parse_env_example(REPO_ROOT / ".env.example")
+    assert set(example) == set(ENV_KEYS.values())
+    assert load_settings(example) == Settings()
+
+
+@pytest.mark.parametrize("forbidden", ["token", "password", "secret", "api_key", "reese", ".com"])
+def test_env_example_values_carry_no_secrets_or_hostnames(forbidden: str) -> None:
+    """The template is committed, so its values must stay placeholders (A16/A17)."""
+    for key, value in parse_env_example(REPO_ROOT / ".env.example").items():
+        assert forbidden not in value.lower(), f"{key} looks like it holds a secret or a real host"
+        assert len(value) <= 40, f"{key} is too long to be a default"
+
+
+# ----------------------------------------------------------------------------- README table
+
+
+def parse_readme_config_table(path: Path) -> dict[str, str]:
+    """Read the VAR / default / meaning rows of the README configuration table.
+
+    A default written as *(empty)* means "no value", i.e. an empty string.
+    """
+    lines = path.read_text(encoding="utf-8").splitlines()
+    try:
+        start = lines.index("## Configuration")
+    except ValueError:  # pragma: no cover - fails loudly if the section is renamed
+        pytest.fail("README.md has no '## Configuration' section")
+    table: dict[str, str] = {}
+    for line in lines[start + 1 :]:
+        if line.startswith("## "):
+            break
+        stripped = line.strip()
+        if not stripped.startswith("|"):
+            continue
+        cells = [cell.strip() for cell in stripped.strip("|").split("|")]
+        key = cells[0].strip("`")
+        if not key.isupper():
+            continue  # header and separator rows
+        default = cells[1].strip("`")
+        table[key] = "" if default.startswith("*") else default
+    return table
+
+
+def test_readme_documents_every_variable_with_its_default() -> None:
+    table = parse_readme_config_table(REPO_ROOT / "README.md")
+    assert set(table) == set(ENV_KEYS.values())
+    assert load_settings(table) == Settings()
+
+
+# ----------------------------------------------------------------------------- CLI overrides
+
+
+def test_apply_cli_overrides_publishes_env_and_rebuilds(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv("UNDERCOOKED_LEVELS_DIR", raising=False)
+    settings = apply_cli_overrides(levels_dir=Path("kitchens"), port=9123)
+    assert settings.levels_dir == Path("kitchens")
+    assert settings.port == 9123
+    assert os.environ["UNDERCOOKED_LEVELS_DIR"] == "kitchens"
+    assert get_settings() == settings  # a later get_settings() sees the same thing
+
+
+def test_apply_cli_overrides_beats_an_existing_environment_value(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("UNDERCOOKED_PORT", "8123")
+    assert apply_cli_overrides(port=9200).port == 9200
+
+
+def test_apply_cli_overrides_validates_the_values_it_writes() -> None:
+    with pytest.raises(ConfigError, match=r"UNDERCOOKED_PORT=0 is invalid"):
+        apply_cli_overrides(port=0)
+
+
+def test_apply_cli_overrides_rejects_an_unknown_field() -> None:
+    with pytest.raises(ConfigError, match="'nonsense' is not a configuration field"):
+        apply_cli_overrides(**{"nonsense": 1})
+
+
+def test_apply_cli_overrides_renders_booleans_as_env_flags() -> None:
+    assert apply_cli_overrides(debugpy=True).debugpy is True
+    assert os.environ["DEBUGPY"] == "1"
+    assert apply_cli_overrides(debugpy=False).debugpy is False
+    assert os.environ["DEBUGPY"] == "0"
+
+
+def test_apply_cli_overrides_with_nothing_still_validates_the_environment(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("UNDERCOOKED_TICK_HZ", "99")
+    with pytest.raises(ConfigError, match="UNDERCOOKED_TICK_HZ=99"):
+        apply_cli_overrides()
+
+
+# ----------------------------------------------------------------------------- conftest factory
+
+
+def test_make_settings_fixture_builds_settings(make_settings: Callable[..., Settings]) -> None:
+    settings = make_settings(tick_hz=30, snapshot_hz=15, debugpy=True, allowed_origins=["http://a", "http://b"])
+    assert (settings.tick_hz, settings.snapshot_hz) == (30, 15)
+    assert settings.debugpy is True
+    assert settings.allowed_origins == ("http://a", "http://b")
+    assert settings.levels_dir == Path("levels")