feat(admin-config-tool): task 2.2 - xcecommon.schema

Load controller.capnp once per process via a cached loader that
resolves identically under editable and pinned installs. Missing or
corrupted schema content converts a pycapnp uncaught-C++-abort into a
clean SchemaNotFoundError via isolated subprocess pre-validation.

.kiro/specs/admin-config-tool/tasks.md

@@ -31,7 +31,7 @@
   - _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 5.5_
   - _Boundary: xcecommon.validation_
 
-- [ ] 2.2 (P) Load the cluster wire-protocol schema
+- [x] 2.2 (P) Load the cluster wire-protocol schema
   - The wire-protocol schema loads once per process and exposes every message type xceadmin needs to build or decode
   - Loading works identically whether xceadmin was installed in editable or pinned mode
   - A missing or corrupted schema copy produces a clear, actionable error rather than a crash
@@ -147,3 +147,4 @@
 - **1.3**: Claude Code `PostToolUse` hooks receive tool input via stdin JSON (`tool_input.file_path`), not a `$FILE_PATH` env var — brief.md's ruff-hook sketch used the env-var form, which would have silently no-op'd. The actual hooks added to `.claude/settings.json` follow this repo's existing `.wolf/hooks/post-write.js` stdin-JSON convention instead. Any future hook work in this repo should follow the same pattern, not brief.md's original sketch.
 - **1.3**: `src/xceadmin/src/xcecommon/schema_files/` (synced capnp + JSON schema copies) is a build artifact, regenerated every `xce-build.sh xceadmin` run — never hand-edit it, never commit it (`src/xceadmin/.gitignore` excludes it).
 - **2.1**: `check_undefined_refs` deliberately does not parse resource names out of `res_req`/`gpu_req` free-form LSF-style expression strings (e.g. `"mem>1000 rusage[mem=1000]"`) — design.md's `xcecommon.validation` component scopes this check to queue→host-group and limit→group/`resname` only. `limits.yaml`'s `projects` field and `scaler.yaml` name collisions are likewise out of scope — no `projects.yaml` catalog exists among the 7 config types, and Requirement 4.3 only enumerates queue/host/resource/group/limit names. `src/configs/hosts.yaml` has 2 pre-existing undefined refs (resource `mg`, host `h3`) that now surface as warnings — expected, not a regression; fixing the example fixture is outside this task's boundary.
+- **2.2**: The real `controller.capnp` names its reply struct `RequestReply`, not `Reply` as design.md's postcondition states — an inexact design pseudo-type, corrected empirically (verified directly against `src/schema/controller.capnp`, not assumed). pycapnp 2.0.0/capnproto 1.2.0 abort the whole process (uncaught C++ exception, not a Python exception) on a missing cross-file import or malformed `.capnp` content passed to `capnp.load()` — a bare `try/except` cannot catch this. `xcecommon.schema.load_schema()` works around it by pre-validating in an isolated subprocess before ever calling `capnp.load()` in the caller's own process, converting any abort into a clean `SchemaNotFoundError`. Any future code that calls `capnp.load()` directly (bypassing this loader) reintroduces the crash risk — always go through `xcecommon.schema.load_schema()`.

src/xceadmin/src/xcecommon/schema.py

@@ -0,0 +1,179 @@
+"""Cap'n Proto wire-protocol schema loader for xcecommon/xceadmin (design.md,
+``xcecommon.schema``; Requirements 5.1, 5.5, 7.4).
+
+This module has no dependency on ``xcecommon.{validation,mapping,wire}``
+(design.md, Dependency Direction) -- those depend on it, not the reverse.
+
+``load_schema()`` is the single load point for ``controller.capnp``: it is
+called once per process (memoized below), and every other module imports
+the returned module object rather than calling ``capnp.load()`` itself.
+This matters beyond "avoid redundant work" -- ``capnp.load()`` returns a
+*new* module object, with non-identical type objects, on every call (
+verified directly: two back-to-back ``capnp.load()`` calls against the
+same file produce ``mod1 is not mod2`` and ``mod1.ClusterConfig is not
+mod2.ClusterConfig``). Without a shared cache, two xcecommon modules each
+calling ``capnp.load()`` independently would hold non-identical
+``ClusterConfig``/``Request`` type objects for what is conceptually the
+same schema.
+
+Crash-avoidance note (this is the reason ``load_schema()`` does more than
+call ``capnp.load()`` directly): pycapnp 2.0.0 / capnproto 1.2.0 do **not**
+raise a catchable Python exception for every schema-loading failure. A
+missing top-level file raises an ordinary ``OSError`` -- but a missing
+cross-file import (``c.capnp`` absent from the same directory) or
+genuinely malformed ``.capnp`` syntax instead propagates as an uncaught
+C++ ``kj::ExceptionImpl``, which crosses the Cython boundary uncaught and
+calls ``std::terminate()`` -- aborting the *entire process* (confirmed
+directly: ``capnp.load()`` against a corrupted file or a schema missing
+its import terminates the interpreter with SIGABRT, not a Python
+exception). The task's Observable bullet ("A missing or corrupted schema
+copy produces a clear, actionable error rather than a crash") is
+unimplementable by wrapping ``capnp.load()`` in a plain ``try/except`` --
+there is nothing in Python to catch. ``load_schema()`` instead:
+
+1. Pre-checks that the resolved path and its ``c.capnp`` sibling import
+   both exist as files, raising :class:`SchemaNotFoundError` (Requirement
+   7.4) *before* ever calling ``capnp.load()`` -- this alone is sufficient
+   for the "not found" failure mode and never risks the abort.
+2. For a schema file that exists but may be corrupted (wrong bytes, a bad
+   copy, truncated content), pre-validates it by attempting the exact same
+   ``capnp.load()`` call inside an isolated subprocess first. A crash
+   there is fully contained -- it takes down the throwaway subprocess, not
+   ours -- and its exit code tells us definitively whether the real,
+   in-process load below is safe to attempt.
+"""
+
+from __future__ import annotations
+
+import functools
+import importlib.resources
+import os
+import subprocess
+import sys
+from pathlib import Path
+from typing import TYPE_CHECKING
+
+import capnp
+
+from xcecommon.errors import SchemaNotFoundError
+
+if TYPE_CHECKING:
+    from types import ModuleType
+
+SCHEMA_PATH_ENV_VAR = "XCEADMIN_SCHEMA_PATH"
+
+_C_CAPNP_FILENAME = "c.capnp"
+_PRECHECK_TIMEOUT_SECONDS = 30
+_PRECHECK_SNIPPET = "import capnp, sys\ncapnp.load(sys.argv[1], imports=[sys.argv[2]])\n"
+
+_INSTALLATION_HINT = f"check your xceadmin installation (override with --schema-path or ${SCHEMA_PATH_ENV_VAR})"
+
+
+def default_schema_path() -> str:
+    """Resolve the path to the bundled ``controller.capnp`` (Requirement
+    5.5).
+
+    ``XCEADMIN_SCHEMA_PATH``, if set, takes precedence -- this is the
+    env-var half of the "--schema-path (CLI) / XCEADMIN_SCHEMA_PATH (env
+    var) override the default for troubleshooting" mechanism (design.md);
+    the CLI-flag half is threaded through as the explicit ``schema_path``
+    argument to :func:`load_schema` by a later task's ``xceadmin.cli``.
+
+    Otherwise uses ``importlib.resources.files("xcecommon") /
+    "schema_files" / "controller.capnp"`` -- the same
+    ``importlib.resources`` mechanism ``xcecommon.validation.
+    default_schema_dir()`` uses for the JSON schemas, resolving correctly
+    under both ``pip install -e .`` (points into the checked-out
+    ``schema_files/`` copy) and ``pip install .`` (points into the
+    installed package's copy) identically.
+    """
+    override = os.environ.get(SCHEMA_PATH_ENV_VAR)
+    if override:
+        return override
+    return str(importlib.resources.files("xcecommon") / "schema_files" / "controller.capnp")
+
+
+def _require_schema_files_present(schema_path: Path) -> None:
+    """Raise :class:`SchemaNotFoundError` if ``schema_path`` or its
+    ``c.capnp`` sibling import is missing -- *before* calling
+    ``capnp.load()`` at all, since a missing import is one of the failure
+    modes that otherwise aborts the process (see module docstring).
+    """
+    if not schema_path.is_file():
+        raise SchemaNotFoundError(
+            f"controller.capnp schema not found at {schema_path} -- {_INSTALLATION_HINT}",
+            path=str(schema_path),
+        )
+
+    c_capnp_path = schema_path.parent / _C_CAPNP_FILENAME
+    if not c_capnp_path.is_file():
+        raise SchemaNotFoundError(
+            f"c.capnp import not found alongside {schema_path} "
+            f"(expected {c_capnp_path}) -- {_INSTALLATION_HINT}",
+            path=str(c_capnp_path),
+        )
+
+
+def _precheck_parses_cleanly(schema_path: Path, import_dir: Path) -> None:
+    """Attempt the real ``capnp.load()`` call inside an isolated
+    subprocess first, so a corrupted schema's process-abort lands on the
+    throwaway subprocess instead of ours (see module docstring). A clean
+    (returncode 0) subprocess exit means the real in-process load that
+    follows is safe.
+
+    Best-effort: if the subprocess itself cannot even be started (e.g. no
+    working Python interpreter, which should not happen in practice since
+    we re-invoke ``sys.executable``), this silently falls through to the
+    real load rather than masking an unrelated environment problem behind
+    a misleading schema error.
+    """
+    try:
+        result = subprocess.run(
+            [sys.executable, "-c", _PRECHECK_SNIPPET, str(schema_path), str(import_dir)],
+            capture_output=True,
+            text=True,
+            timeout=_PRECHECK_TIMEOUT_SECONDS,
+        )
+    except (OSError, subprocess.TimeoutExpired):
+        return
+
+    if result.returncode != 0:
+        detail = (result.stderr or result.stdout or "unknown schema parse error").strip()
+        raise SchemaNotFoundError(
+            f"controller.capnp schema at {schema_path} is corrupted or cannot be "
+            f"parsed: {detail} -- {_INSTALLATION_HINT}",
+            path=str(schema_path),
+        )
+
+
+@functools.lru_cache(maxsize=None)
+def _load_schema_cached(schema_path: str) -> "ModuleType":
+    path = Path(schema_path)
+    _require_schema_files_present(path)
+    _precheck_parses_cleanly(path, path.parent)
+    return capnp.load(str(path), imports=[str(path.parent)])
+
+
+def load_schema(schema_path: str | None = None) -> "ModuleType":
+    """Load ``controller.capnp`` (``default_schema_path()`` if
+    ``schema_path`` is ``None``), returning the capnp-generated module
+    exposing ``.ClusterConfig``, ``.Request``, ``.RequestReply`` (the
+    reply-message struct -- design.md's Service Interface calls this
+    ``.Reply``, but the real schema names it ``RequestReply``; verified
+    directly against ``capnp.load()`` output, not assumed),
+    ``.UpgradeConfigRequest``/``.UpgradeConfigReply``,
+    ``.ProbeConfigTagRequest``/``.ProbeConfigTagReply``, and every other
+    top-level type ``controller.capnp`` declares (Requirement 5.1).
+
+    Memoized per resolved path -- repeated calls with the same
+    ``schema_path`` (including repeated implicit-default calls) return the
+    exact same module object, never re-loading (design.md: "Single load
+    point ... called once per process").
+
+    Raises :class:`SchemaNotFoundError` if the resolved path or its
+    ``c.capnp`` import is missing, or if the schema content is corrupted
+    and cannot be parsed (Requirement 7.4) -- never lets a raw capnp
+    exception or process abort escape.
+    """
+    resolved_path = schema_path if schema_path is not None else default_schema_path()
+    return _load_schema_cached(resolved_path)

src/xceadmin/tests/test_schema.py

@@ -0,0 +1,270 @@
+"""Tests for xcecommon.schema (task 2.2).
+
+Covers Requirements 5.1, 5.5, 7.4: loading the bundled ``controller.capnp``
+wire-protocol schema once per process and exposing every message type
+xceadmin needs to build or decode, resolving identically under both
+editable and pinned (non-editable) installs, and producing a clear,
+actionable error -- never a crash -- when the bundled schema copy is
+missing or corrupted.
+
+Fixture strategy mirrors task 2.1's ``test_validation.py``: the "happy
+path" tests exercise the *real* bundled schema (``default_schema_path()``)
+so we prove the actual shipped ``controller.capnp``/``c.capnp`` pair loads
+and exposes real top-level types, not just a synthetic fixture. The
+missing/corrupted-schema error-path tests build small throwaway ``.capnp``
+files under ``tmp_path`` instead, since those failure modes must not touch
+the real bundled copy.
+
+Design-vs-reality note (see schema.py module docstring for the full
+explanation): design.md's Service Interface postcondition lists a
+``Reply`` top-level type, but the real ``controller.capnp`` names this
+struct ``RequestReply`` (``RequestReply`` is the reply *message*;
+``request.body.replymsg`` embeds it). Verified directly against
+``capnp.load()`` output rather than assumed -- see
+``test_load_schema_exposes_expected_top_level_types`` below, which asserts
+against the real name.
+"""
+
+from __future__ import annotations
+
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+from xcecommon.errors import SchemaNotFoundError
+from xcecommon.schema import (
+    SCHEMA_PATH_ENV_VAR,
+    default_schema_path,
+    load_schema,
+)
+
+# ---------------------------------------------------------------------------
+# default_schema_path() -- resolution under the active install
+# ---------------------------------------------------------------------------
+
+
+def test_default_schema_path_resolves_to_real_bundled_schema() -> None:
+    """default_schema_path() must resolve to a real, existing
+    controller.capnp, with c.capnp sitting alongside it (Requirement 5.5).
+    This is the assertion that matters identically whether xceadmin is
+    installed editable or pinned -- see module docstring and the task's
+    Observable bullet; the *editable-vs-pinned* half of that guarantee is
+    proven by running this same test suite under both
+    `xce-build.sh xceadmin` and `xce-build.sh xceadmin release` profiles
+    (see TESTS_RUN in the task status report), not by anything expressible
+    inside a single test process.
+    """
+    schema_path = Path(default_schema_path())
+
+    assert schema_path.is_file()
+    assert schema_path.name == "controller.capnp"
+    assert (schema_path.parent / "c.capnp").is_file()
+
+
+def test_default_schema_path_env_var_override(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    """XCEADMIN_SCHEMA_PATH overrides the packaged default, per design.md's
+    '--schema-path (CLI) / XCEADMIN_SCHEMA_PATH (env var) override the
+    default for troubleshooting' (7.4)."""
+    custom = tmp_path / "custom.capnp"
+    monkeypatch.setenv(SCHEMA_PATH_ENV_VAR, str(custom))
+
+    assert default_schema_path() == str(custom)
+
+
+def test_default_schema_path_no_env_var_uses_packaged_default(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv(SCHEMA_PATH_ENV_VAR, raising=False)
+
+    resolved = Path(default_schema_path())
+
+    assert resolved.is_file()
+
+
+# ---------------------------------------------------------------------------
+# load_schema() -- happy path against the real bundled schema
+# ---------------------------------------------------------------------------
+
+
+def test_load_schema_default_exposes_expected_top_level_types() -> None:
+    """load_schema() with no argument must load the real bundled schema
+    and expose every top-level type xceadmin needs to build or decode
+    (Requirement 5.1): ClusterConfig plus its nested list-section types,
+    Request, the real reply struct name RequestReply (not "Reply" -- see
+    module docstring), and the Upgrade/ProbeConfigTag request/reply pairs
+    used by config push / --dry-run."""
+    mod = load_schema()
+
+    for type_name in (
+        "ClusterConfig",
+        "LimitConfig",
+        "HostConfig",
+        "ScalerConfig",
+        "Resource",
+        "QueueConf",
+        "GroupConfig",
+        "Request",
+        "RequestReply",
+        "UpgradeConfigRequest",
+        "UpgradeConfigReply",
+        "ProbeConfigTagRequest",
+        "ProbeConfigTagReply",
+        "OPCode",
+    ):
+        assert hasattr(mod, type_name), f"loaded schema module missing {type_name}"
+
+
+def test_load_schema_default_types_are_usable_messages() -> None:
+    """The exposed types must be real, constructible capnp struct types --
+    not just names -- proving the schema actually parsed and linked
+    correctly against the real bundled controller.capnp/c.capnp pair."""
+    mod = load_schema()
+
+    cluster_config = mod.ClusterConfig.new_message()
+    cluster_config.clusterName = "test-cluster"
+    assert cluster_config.clusterName == "test-cluster"
+
+    assert "upgradeConfig" in mod.OPCode.schema.enumerants
+
+
+def test_load_schema_caches_default_load_returns_same_module_object() -> None:
+    """'Single load point ... called once per process; other modules
+    import the returned module object, never re-load' (design.md,
+    xcecommon.schema Responsibilities & Constraints). capnp.load() itself
+    returns a *new* module object (and new type objects) on every call --
+    verified directly -- so callers that didn't go through a shared cache
+    would silently get non-identical ClusterConfig/Request type objects
+    across modules. load_schema() must paper over that."""
+    first = load_schema()
+    second = load_schema()
+
+    assert first is second
+    assert first.ClusterConfig is second.ClusterConfig
+
+
+def test_load_schema_explicit_path_matching_default_is_usable() -> None:
+    """An explicit schema_path pointing at the same real bundled file
+    (the --schema-path override path, at the xcecommon.schema layer)
+    loads just as well as the implicit default."""
+    mod = load_schema(default_schema_path())
+
+    assert hasattr(mod, "ClusterConfig")
+
+
+# ---------------------------------------------------------------------------
+# load_schema() -- missing schema (Requirement 7.4)
+# ---------------------------------------------------------------------------
+
+
+def test_load_schema_missing_file_raises_schema_not_found_error(tmp_path: Path) -> None:
+    """A nonexistent controller.capnp path must raise SchemaNotFoundError
+    with the missing path attached (Requirement 7.4) -- and must never
+    reach capnp.load() itself: an absent file passed to capnp.load()
+    (without the c.capnp sibling present either) is a second, compounding
+    failure mode covered separately below."""
+    missing = tmp_path / "does-not-exist.capnp"
+
+    with pytest.raises(SchemaNotFoundError) as exc_info:
+        load_schema(str(missing))
+
+    assert exc_info.value.path == str(missing)
+    assert str(missing) in exc_info.value.message
+
+
+def test_load_schema_missing_c_capnp_import_raises_schema_not_found_error(
+    tmp_path: Path,
+) -> None:
+    """controller.capnp present but its c.capnp sibling import missing --
+    e.g. a broken/partial installation -- must raise SchemaNotFoundError,
+    not the uncatchable process abort pycapnp/kj otherwise produces for a
+    failed import (verified empirically, see module docstring). This is
+    the 'installation is broken' reading of the task's 'corrupted schema
+    copy' bullet."""
+    real_schema = Path(default_schema_path())
+    broken = tmp_path / "controller.capnp"
+    broken.write_text(real_schema.read_text(encoding="utf-8"), encoding="utf-8")
+    # Deliberately do NOT copy c.capnp alongside it.
+
+    with pytest.raises(SchemaNotFoundError) as exc_info:
+        load_schema(str(broken))
+
+    assert "c.capnp" in exc_info.value.message
+
+
+def test_load_schema_env_var_override_missing_raises_schema_not_found_error(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    missing = tmp_path / "nope.capnp"
+    monkeypatch.setenv(SCHEMA_PATH_ENV_VAR, str(missing))
+
+    with pytest.raises(SchemaNotFoundError):
+        load_schema()
+
+
+# ---------------------------------------------------------------------------
+# load_schema() -- corrupted (present but invalid) schema content
+# ---------------------------------------------------------------------------
+
+
+def test_load_schema_corrupted_content_raises_schema_not_found_error_not_crash(
+    tmp_path: Path,
+) -> None:
+    """A present-but-syntactically-invalid controller.capnp must raise a
+    clear SchemaNotFoundError, not crash the process.
+
+    This is the test that would fail loudly (by taking the whole pytest
+    process down with it, exit via SIGABRT) if load_schema() called
+    capnp.load() directly on unvalidated content: pycapnp 2.0.0 /
+    capnproto 1.2.0 propagate a genuine capnp parse error as an uncaught
+    C++ 'kj::ExceptionImpl', which calls std::terminate() before it ever
+    reaches Python's exception machinery -- confirmed by direct
+    experimentation, not assumed. load_schema() must pre-validate in an
+    isolated subprocess before ever loading in-process."""
+    broken = tmp_path / "controller.capnp"
+    broken.write_text("this is not { valid capnp syntax at all ]]]\n", encoding="utf-8")
+    real_schema = Path(default_schema_path())
+    (tmp_path / "c.capnp").write_text(
+        (real_schema.parent / "c.capnp").read_text(encoding="utf-8"), encoding="utf-8"
+    )
+
+    with pytest.raises(SchemaNotFoundError) as exc_info:
+        load_schema(str(broken))
+
+    assert str(broken) in exc_info.value.message
+
+
+def test_load_schema_corrupted_content_subprocess_never_aborts(tmp_path: Path) -> None:
+    """Belt-and-suspenders proof, fully isolated from pytest's own
+    process: run load_schema() against a corrupted schema inside a
+    throwaway subprocess and assert it exits with an ordinary Python
+    exception (returncode 1 from an uncaught SchemaNotFoundError formatted
+    by the interpreter), never a negative returncode (== killed by a
+    signal, e.g. -6/SIGABRT from the raw capnp.load() abort this task
+    exists to prevent)."""
+    broken = tmp_path / "controller.capnp"
+    broken.write_text("also not valid capnp {{{\n", encoding="utf-8")
+    real_schema = Path(default_schema_path())
+    (tmp_path / "c.capnp").write_text(
+        (real_schema.parent / "c.capnp").read_text(encoding="utf-8"), encoding="utf-8"
+    )
+
+    snippet = (
+        "from xcecommon.schema import load_schema\n"
+        "import sys\n"
+        "load_schema(sys.argv[1])\n"
+    )
+    result = subprocess.run(
+        [sys.executable, "-c", snippet, str(broken)],
+        capture_output=True,
+        text=True,
+        timeout=30,
+    )
+
+    assert result.returncode >= 0, (
+        f"load_schema() let a raw process abort escape (returncode="
+        f"{result.returncode}); stderr={result.stderr!r}"
+    )
+    assert result.returncode != 0
+    assert "SchemaNotFoundError" in result.stderr