From 505d8bae5a99fcb77ece4e9655c376344a1eef01 Mon Sep 17 00:00:00 2001 From: Kenny Van de Maele Date: Mon, 8 Jun 2026 01:13:47 +0200 Subject: [PATCH] fix(cookbook): locate cookbook_state.json via DATA_DIR, not hardcoded /app/data (#3332) Three call sites hardcoded Path("/app/data/cookbook_state.json"), which only exists in Docker; on a native run the real path is /data, so the state file looked missing and cookbook serve-state was silently ignored. Two others used os.environ.get("DATA_DIR", "data") (a relative fallback, since DATA_DIR is never set as an env var). Route all five through core.constants.DATA_DIR so the path is consistent and absolute on both Docker and native. Part of #3331. Co-authored-by: Claude Opus 4.8 --- routes/codex_routes.py | 5 +++-- routes/cookbook_routes.py | 3 ++- src/builtin_actions.py | 4 ++-- src/cookbook_serve_lifecycle.py | 4 ++-- tests/test_cookbook_state_path.py | 29 +++++++++++++++++++++++++++++ 5 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 tests/test_cookbook_state_path.py diff --git a/routes/codex_routes.py b/routes/codex_routes.py index c641c3915..e1f3b49c0 100644 --- a/routes/codex_routes.py +++ b/routes/codex_routes.py @@ -17,6 +17,7 @@ from fastapi.responses import StreamingResponse from src.auth_helpers import require_authenticated_request, require_user from src.tool_implementations import do_manage_notes +from core.constants import DATA_DIR COOKBOOK_READ_SCOPES = {"cookbook:read", "cookbook:launch"} @@ -425,7 +426,7 @@ def setup_codex_routes( def _read_cookbook_state() -> dict: from pathlib import Path as _Path import os as _os, json as _json - p = _Path(_os.environ.get("DATA_DIR", "data")) / "cookbook_state.json" + p = _Path(DATA_DIR) / "cookbook_state.json" if not p.exists(): return {} try: @@ -733,7 +734,7 @@ def setup_codex_routes( import time as _t, json as _json from core.atomic_io import atomic_write_json from pathlib import Path as _Path - cookbook_state_path = _Path("/app/data/cookbook_state.json") + cookbook_state_path = _Path(DATA_DIR) / "cookbook_state.json" try: state = _json.loads(cookbook_state_path.read_text(encoding="utf-8")) except Exception: diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index 84ec80a71..5974526eb 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -33,6 +33,7 @@ from core.platform_compat import ( get_wsl_windows_user_profile, ) from routes.shell_routes import TMUX_LOG_DIR +from core.constants import DATA_DIR logger = logging.getLogger(__name__) @@ -60,7 +61,7 @@ _HF_TOKEN_STATUS_SNIPPET = ( def setup_cookbook_routes() -> APIRouter: router = APIRouter(tags=["cookbook"]) - _cookbook_state_path = Path(os.environ.get("DATA_DIR", "data")) / "cookbook_state.json" + _cookbook_state_path = Path(DATA_DIR) / "cookbook_state.json" def _mask_secret(value: str) -> str: if not value: diff --git a/src/builtin_actions.py b/src/builtin_actions.py index a6c535a3a..1e0c681fe 100644 --- a/src/builtin_actions.py +++ b/src/builtin_actions.py @@ -12,7 +12,7 @@ from typing import Tuple from src.auth_helpers import owner_filter from core.platform_compat import IS_WINDOWS, find_bash -from core.constants import internal_api_base +from core.constants import DATA_DIR, internal_api_base logger = logging.getLogger(__name__) @@ -2043,7 +2043,7 @@ async def action_cookbook_serve( except Exception: end_after_min = 0 - state_path = Path("/app/data/cookbook_state.json") + state_path = Path(DATA_DIR) / "cookbook_state.json" try: state = json.loads(state_path.read_text(encoding="utf-8")) if state_path.exists() else {} except Exception: diff --git a/src/cookbook_serve_lifecycle.py b/src/cookbook_serve_lifecycle.py index 180313034..6948763b7 100644 --- a/src/cookbook_serve_lifecycle.py +++ b/src/cookbook_serve_lifecycle.py @@ -19,7 +19,7 @@ import time from pathlib import Path import httpx -from core.constants import internal_api_base +from core.constants import DATA_DIR, internal_api_base logger = logging.getLogger(__name__) @@ -130,7 +130,7 @@ async def _stop_serve(session_id: str, remote_host: str = "", ssh_port: str = "" async def _tick() -> None: - state_path = Path("/app/data/cookbook_state.json") + state_path = Path(DATA_DIR) / "cookbook_state.json" if not state_path.exists(): return try: diff --git a/tests/test_cookbook_state_path.py b/tests/test_cookbook_state_path.py new file mode 100644 index 000000000..eb239988e --- /dev/null +++ b/tests/test_cookbook_state_path.py @@ -0,0 +1,29 @@ +"""Guard: cookbook_state.json must be located via DATA_DIR, not hardcoded /app/data +(which breaks native runs) or a relative os.environ fallback.""" +import pathlib + +ROOT = pathlib.Path(__file__).resolve().parent.parent +FILES = [ + "src/cookbook_serve_lifecycle.py", + "src/builtin_actions.py", + "routes/codex_routes.py", + "routes/cookbook_routes.py", +] + + +def test_no_hardcoded_app_data_cookbook_state(): + for rel in FILES: + text = (ROOT / rel).read_text(encoding="utf-8") + for ln in text.splitlines(): + if ln.strip().startswith("#"): + continue + assert "/app/data/cookbook_state" not in ln, f"{rel}: hardcoded /app/data: {ln.strip()}" + assert 'os.environ.get("DATA_DIR"' not in ln, f"{rel}: relative DATA_DIR env fallback: {ln.strip()}" + + +def test_cookbook_state_uses_datadir_constant(): + # Each file that references cookbook_state.json should import the DATA_DIR constant. + for rel in FILES: + text = (ROOT / rel).read_text(encoding="utf-8") + if "cookbook_state.json" in text: + assert "from core.constants import DATA_DIR" in text, f"{rel}: missing DATA_DIR import"