feat(core): single-source data_dir/repos_dir via castle.yaml
The `castle` CLI and the `castle-api` service are two independent in-process drivers of `castle_core`. Each resolved DATA_DIR/REPOS_DIR at import time from its own process env (default /data/castle), persisted nowhere — so they silently diverged, and `apply`/dashboard-apply crashed on a non-existent /data. Make the loaded CastleConfig the single source of truth: - Resolve data_dir/repos_dir only in load_config (env > castle.yaml > default), anchored to the config root; drop the DATA_DIR/REPOS_DIR module globals and the import-time file read entirely — no global twin that can disagree with the file. - Thread config.data_dir/repos_dir through ensure_dirs, _env_context, tls_dir_for (now unified — deploy no longer inlines the tls path), and create/add/clone. - ensure_dirs raises an actionable CastleDirError instead of a bare PermissionError; the api surfaces it as 422. - doctor: "data dir writable" check + WARN when CASTLE_DATA_DIR/REPOS_DIR env overrides the file (the one remaining cross-process divergence vector). - install.sh persists data_dir/repos_dir into castle.yaml (idempotent, non-default). - Docs: registry.md globals + AGENTS.md roots. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -11,7 +11,7 @@ import argparse
|
||||
import tomllib
|
||||
from pathlib import Path
|
||||
|
||||
from castle_cli.config import REPOS_DIR, load_config, save_config
|
||||
from castle_cli.config import load_config, save_config
|
||||
from castle_cli.manifest import BuildSpec, CommandsSpec, ProgramSpec
|
||||
|
||||
|
||||
@@ -74,7 +74,7 @@ def run_add(args: argparse.Namespace) -> int:
|
||||
repo_url = target
|
||||
name = args.name or Path(target.rstrip("/")).name.removesuffix(".git")
|
||||
# Default local clone location; cloned later via `castle clone`.
|
||||
source = str(REPOS_DIR / name)
|
||||
source = str(config.repos_dir / name)
|
||||
src_path = Path(source)
|
||||
else:
|
||||
src_path = Path(target).expanduser().resolve()
|
||||
|
||||
@@ -10,11 +10,11 @@ import argparse
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from castle_cli.config import REPOS_DIR, load_config
|
||||
from castle_cli.config import load_config
|
||||
|
||||
|
||||
def _clone_one(name: str, repo: str, source: str | None, ref: str | None) -> bool:
|
||||
dest = Path(source) if source else REPOS_DIR / name
|
||||
def _clone_one(name: str, repo: str, source: str | None, ref: str | None, repos_dir: Path) -> bool:
|
||||
dest = Path(source) if source else repos_dir / name
|
||||
if dest.exists():
|
||||
print(f" {name}: already present at {dest}, skipping")
|
||||
return True
|
||||
@@ -46,7 +46,7 @@ def run_clone(args: argparse.Namespace) -> int:
|
||||
if not prog.repo:
|
||||
print(f"{args.name} has no repo: URL to clone from")
|
||||
return 1
|
||||
return 0 if _clone_one(args.name, prog.repo, prog.source, prog.ref) else 1
|
||||
return 0 if _clone_one(args.name, prog.repo, prog.source, prog.ref, config.repos_dir) else 1
|
||||
|
||||
# Clone all programs that declare a repo: and lack a present source.
|
||||
all_ok = True
|
||||
@@ -55,7 +55,7 @@ def run_clone(args: argparse.Namespace) -> int:
|
||||
if not prog.repo:
|
||||
continue
|
||||
cloned_any = True
|
||||
if not _clone_one(name, prog.repo, prog.source, prog.ref):
|
||||
if not _clone_one(name, prog.repo, prog.source, prog.ref, config.repos_dir):
|
||||
all_ok = False
|
||||
if not cloned_any:
|
||||
print("No programs declare a repo: URL.")
|
||||
|
||||
@@ -5,7 +5,7 @@ from __future__ import annotations
|
||||
import argparse
|
||||
import subprocess
|
||||
|
||||
from castle_cli.config import REPOS_DIR, load_config, save_config
|
||||
from castle_cli.config import load_config, save_config
|
||||
from castle_cli.manifest import (
|
||||
BuildSpec,
|
||||
CaddyDeployment,
|
||||
@@ -75,8 +75,8 @@ def run_create(args: argparse.Namespace) -> int:
|
||||
print(f"Error: '{name}' already exists in castle.yaml")
|
||||
return 1
|
||||
|
||||
REPOS_DIR.mkdir(parents=True, exist_ok=True)
|
||||
project_dir = REPOS_DIR / name
|
||||
config.repos_dir.mkdir(parents=True, exist_ok=True)
|
||||
project_dir = config.repos_dir / name
|
||||
if project_dir.exists():
|
||||
print(f"Error: directory already exists: {project_dir}")
|
||||
return 1
|
||||
|
||||
@@ -12,6 +12,7 @@ doubles as a scriptable smoke test after `./install.sh` or `castle apply`.
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import shutil
|
||||
import socket
|
||||
from dataclasses import dataclass
|
||||
@@ -136,6 +137,37 @@ def _check_configuration(config) -> list[Check]:
|
||||
)
|
||||
)
|
||||
|
||||
# data dir must exist and be writable — the exact condition that crashes apply
|
||||
# (ensure_dirs) when data_dir points at a non-existent volume like /data.
|
||||
ddir = config.data_dir
|
||||
if ddir.is_dir() and os.access(ddir, os.W_OK):
|
||||
checks.append(Check(OK, "data dir writable", detail=str(ddir)))
|
||||
else:
|
||||
checks.append(
|
||||
Check(
|
||||
FAIL,
|
||||
"data dir missing or not writable",
|
||||
detail=str(ddir),
|
||||
hint=f"set data_dir: in ~/.castle/castle.yaml, or: "
|
||||
f"sudo mkdir -p {ddir} && sudo chown $(id -un) {ddir}",
|
||||
)
|
||||
)
|
||||
|
||||
# Drift guard: castle.yaml is the single source of truth for the roots. An env var
|
||||
# override is per-process, so it's the one way the CLI and the api service can still
|
||||
# diverge (env set in your shell, absent in the service unit — the original bug).
|
||||
for var in ("CASTLE_DATA_DIR", "CASTLE_REPOS_DIR"):
|
||||
if var in os.environ:
|
||||
checks.append(
|
||||
Check(
|
||||
WARN,
|
||||
f"{var} overrides castle.yaml",
|
||||
detail=f"{var}={os.environ[var]}",
|
||||
hint=f"set data_dir:/repos_dir: in castle.yaml and unset {var}, so "
|
||||
"every process (CLI and api) resolves the same roots",
|
||||
)
|
||||
)
|
||||
|
||||
missing = [n for n in (_GATEWAY, _API, _DASHBOARD) if not config.deployments_named(n)]
|
||||
if not missing:
|
||||
checks.append(Check(OK, "control plane registered", detail="gateway, api, dashboard"))
|
||||
|
||||
@@ -6,9 +6,7 @@ from castle_core.config import ( # noqa: F401 — explicit re-exports for type
|
||||
CASTLE_HOME,
|
||||
CODE_DIR,
|
||||
CONTENT_DIR,
|
||||
DATA_DIR,
|
||||
GENERATED_DIR,
|
||||
REPOS_DIR,
|
||||
SECRETS_DIR,
|
||||
SPECS_DIR,
|
||||
STATIC_DIR,
|
||||
|
||||
@@ -18,9 +18,9 @@ class TestCreateCommand:
|
||||
with (
|
||||
patch("castle_cli.commands.create.load_config") as mock_load,
|
||||
patch("castle_cli.commands.create.save_config") as mock_save,
|
||||
patch("castle_cli.commands.create.REPOS_DIR", repos),
|
||||
):
|
||||
config = load_config(castle_root)
|
||||
config.repos_dir = repos
|
||||
mock_load.return_value = config
|
||||
|
||||
from castle_cli.commands.create import run_create
|
||||
@@ -57,9 +57,9 @@ class TestCreateCommand:
|
||||
with (
|
||||
patch("castle_cli.commands.create.load_config") as mock_load,
|
||||
patch("castle_cli.commands.create.save_config"),
|
||||
patch("castle_cli.commands.create.REPOS_DIR", repos),
|
||||
):
|
||||
config = load_config(castle_root)
|
||||
config.repos_dir = repos
|
||||
mock_load.return_value = config
|
||||
|
||||
from castle_cli.commands.create import run_create
|
||||
@@ -84,9 +84,9 @@ class TestCreateCommand:
|
||||
with (
|
||||
patch("castle_cli.commands.create.load_config") as mock_load,
|
||||
patch("castle_cli.commands.create.save_config"),
|
||||
patch("castle_cli.commands.create.REPOS_DIR", repos),
|
||||
):
|
||||
config = load_config(castle_root)
|
||||
config.repos_dir = repos
|
||||
mock_load.return_value = config
|
||||
|
||||
from castle_cli.commands.create import run_create
|
||||
@@ -138,9 +138,9 @@ class TestCreateCommand:
|
||||
with (
|
||||
patch("castle_cli.commands.create.load_config") as mock_load,
|
||||
patch("castle_cli.commands.create.save_config"),
|
||||
patch("castle_cli.commands.create.REPOS_DIR", tmp_path / "repos"),
|
||||
):
|
||||
config = load_config(castle_root)
|
||||
config.repos_dir = tmp_path / "repos"
|
||||
mock_load.return_value = config
|
||||
|
||||
from castle_cli.commands.create import run_create
|
||||
|
||||
@@ -6,7 +6,8 @@ from argparse import Namespace
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
from castle_cli.commands.doctor import run_doctor
|
||||
import pytest
|
||||
from castle_cli.commands.doctor import FAIL, OK, WARN, _check_configuration, run_doctor
|
||||
|
||||
|
||||
class TestDoctor:
|
||||
@@ -37,3 +38,50 @@ class TestDoctor:
|
||||
out = capsys.readouterr().out # type: ignore[attr-defined]
|
||||
assert "failed to load" in out
|
||||
assert "bad yaml" in out
|
||||
|
||||
|
||||
class TestDataDirChecks:
|
||||
"""The drift-prevention checks: data_dir must be writable, and a CASTLE_DATA_DIR env
|
||||
override (the one way the CLI and api can still diverge) must be surfaced."""
|
||||
|
||||
def _config(self, castle_root: Path):
|
||||
from castle_cli.config import load_config
|
||||
|
||||
return load_config(castle_root)
|
||||
|
||||
def test_writable_dir_ok_no_warn(
|
||||
self, castle_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.delenv("CASTLE_DATA_DIR", raising=False)
|
||||
monkeypatch.delenv("CASTLE_REPOS_DIR", raising=False)
|
||||
cfg = self._config(castle_root)
|
||||
cfg.data_dir = tmp_path # exists + writable
|
||||
checks = _check_configuration(cfg)
|
||||
by_label = {c.label: c for c in checks}
|
||||
assert by_label["data dir writable"].status == OK
|
||||
assert not any("overrides castle.yaml" in c.label for c in checks)
|
||||
|
||||
def test_missing_dir_fails_with_hint(
|
||||
self, castle_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.delenv("CASTLE_DATA_DIR", raising=False)
|
||||
cfg = self._config(castle_root)
|
||||
missing = tmp_path / "nope"
|
||||
cfg.data_dir = missing
|
||||
fail = next(
|
||||
c for c in _check_configuration(cfg) if "data dir" in c.label and c.status == FAIL
|
||||
)
|
||||
assert str(missing) in fail.detail
|
||||
assert fail.hint # offers a concrete fix
|
||||
|
||||
def test_env_override_warns(
|
||||
self, castle_root: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A CASTLE_DATA_DIR env var overrides the single-source-of-truth file — the
|
||||
exact CLI/api divergence we fixed. Doctor must WARN."""
|
||||
monkeypatch.setenv("CASTLE_DATA_DIR", str(tmp_path))
|
||||
cfg = self._config(castle_root)
|
||||
cfg.data_dir = tmp_path
|
||||
warn = next(c for c in _check_configuration(cfg) if "overrides castle.yaml" in c.label)
|
||||
assert warn.status == WARN
|
||||
assert "CASTLE_DATA_DIR" in warn.detail
|
||||
|
||||
Reference in New Issue
Block a user