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

# Implementation Plan

- [ ] 1. Foundation: package scaffolding, shared errors, build/toolchain integration
- [x] 1.1 Set up the xceadmin Python package skeleton
  - Editable install registers a real `xceadmin` console-script entry point backed by an empty, working click group (no subcommands yet — `config build`/`config push` are added by task 4.1, not this task)
  - `xcecommon` is importable as a standalone package independent of `xceadmin`'s CLI
  - Observable: running the entry point after an editable install prints usage/help text without error, showing zero commands
  - _Requirements: 5.4_

- [x] 1.2 Establish a shared error-handling foundation
  - One common base exception type carries a human-readable message
  - Every specific failure mode (schema not found, validation failure, reserved name, duplicate entry, connection timeout, corrupt reply, too many redirects, no primary available) has its own subclass
  - Observable: raising any of these subclasses and catching the common base type recovers the message
  - _Requirements: 7.1, 7.2, 7.3, 7.4_

- [x] 1.3 Wire xceadmin into the project's build and dev-shell tooling
  - Nix dev shell provides the Python interpreter and all dependencies needed to build and test xceadmin
  - Running the build command for xceadmin performs an editable install by default and a pinned non-editable install under the release profile, running the test suite after whichever install just happened
  - The build step keeps the packaged copies of the wire-protocol schema and the validation schemas in sync with their source locations elsewhere in the repo, and packages both correctly under either install mode
  - Build/test command documentation, permission allowlist entries, and automatic formatting/linting on Python file edits all exist for the new component
  - Observable: running the build command for this component from a clean checkout succeeds and reports test pass/fail
  - _Requirements: 5.5, 8.1, 8.2, 8.6_

- [ ] 2. Core: xcecommon shared library
- [x] 2.1 (P) Validate operator-authored YAML against the bundled schemas
  - Each of the seven config file types can be checked against its own bundled validation schema, independent of anything the operator supplies
  - Reserved queue/host names are rejected even when otherwise schema-valid
  - Duplicate names within a single file/section are rejected
  - Cross-references to undefined groups or resources are flagged as warnings without blocking the build
  - Observable: validating a config directory with no schema files present anywhere in it still succeeds
  - _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 5.5_
  - _Boundary: xcecommon.validation_

- [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
  - Observable: loading the schema under a pinned (non-editable) install still resolves the bundled copy correctly
  - _Requirements: 5.1, 5.5, 7.4_
  - _Boundary: xcecommon.schema_

- [x] 2.3 (P) Map validated YAML into cluster configuration fields
  - Every field the cluster configuration binary format defines is either populated from validated YAML or deliberately left untouched, with no field silently dropped
  - List-based sections preserve the exact order they appeared in the YAML
  - No timestamp, hostname, or username value ever enters the mapped output
  - Building the same input twice produces identical output
  - _Requirements: 1.1, 1.7, 1.8, 1.9, 1.10, 1.11, 1.12, 5.3, 9.1, 9.2, 9.3_
  - _Boundary: xcecommon.mapping_
  - _Depends: 2.2_

- [x] 2.4 (P) Establish the TLS connection and wire framing to the controller
  - A single request can be sent and a single reply received over a TLS connection using the cluster's existing certificate trust model
  - Message framing matches the controller's own byte-for-byte, verified against known-good byte sequences
  - The connection always closes cleanly, including on error or interruption
  - _Requirements: 2.1, 2.3, 2.4, 5.2, 6.1, 7.1, 7.3, 7.6_
  - _Boundary: xcecommon.wire_
  - _Depends: 2.2_

- [ ] 3. Core: xceadmin CLI orchestration
- [x] 3.1 (P) Compile a configs directory into a deployable cluster binary
  - Running the build command against a directory of YAML files produces a binary output file with a printed checksum
  - A missing individual YAML file is treated as an empty section, not an error
  - Unrecognized-but-valid YAML fields produce a warning, not a failure
  - Output can be directed to a specific path or to standard output
  - _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.9, 8.3, 8.4_
  - _Boundary: xceadmin.build_
  - _Depends: 2.1, 2.3_

- [x] 3.2 (P) Follow controller redirects to find the primary
  - A reply pointing at a different host/port is followed automatically, up to a bounded number of hops
  - Exceeding the hop limit or waiting past the connection timeout produces a clear error naming the last host contacted
  - Observable: a chain of up to three redirects resolves to a final destination without operator intervention
  - _Requirements: 3.1, 3.2, 3.3, 3.4_
  - _Boundary: xceadmin.discovery_
  - _Depends: 2.4_

- [x] 3.3 Push a cluster binary to the controller, with a dry-run mode
  - A real push reports the assigned configuration version, acknowledgement count, and any error, exiting non-zero on failure
  - A dry-run reports whether the controller would accept the config without ever committing it
  - Every connection failure, protocol-level rejection, corrupt input, or missing schema maps to a specific, distinct error
  - _Requirements: 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 6.1, 6.2, 6.3, 7.1, 7.2, 7.3, 7.4, 7.5, 7.6_
  - _Boundary: xceadmin.push_
  - _Depends: 2.4, 3.2_

- [ ] 4. Integration: wire the CLI entry point and hook xceadmin into cluster deployment
- [x] 4.1 Expose config build and config push as a single command-line tool
  - `xceadmin config build` and `xceadmin config push` (with `--dry-run`) are both reachable from one installed command
  - Any failure surfaces as a specific, human-readable message with a non-zero exit code, not a raw traceback
  - Interrupting a push mid-flight leaves no dangling connection and reports a clean cancellation
  - _Requirements: 1.2, 1.3, 2.2, 2.4, 7.1, 7.2, 7.3, 7.4, 7.5, 7.6, 8.1, 8.2_
  - _Boundary: xceadmin.cli_
  - _Depends: 3.1, 3.3_

- [x] 4.2 Build cluster configuration automatically during HA deployment
  - Deploying a cluster with the build-config option produces a fresh binary from the current YAML before startup
  - Deploying with no binary present and no flag still produces one automatically
  - Deploying with an existing binary and no flag leaves it untouched, matching prior behavior
  - _Requirements: 8.3, 8.4, 8.5_
  - _Boundary: deploy-ha.sh_
  - _Depends: 4.1_

- [ ] 5. Validation: test coverage across build, push, and packaging
- [x] 5.1 (P) Unit test the field-mapping logic in isolation
  - Both mapper shapes (list sections and the scalar section) are verified against in-memory input with no file I/O
  - The one file that feeds two separate output fields is verified explicitly
  - _Requirements: 1.1, 1.7, 1.8, 1.9, 1.10, 1.11, 1.12, 5.3, 9.1, 9.2, 9.3_
  - _Boundary: xcecommon.mapping_
  - _Depends: 2.3_

- [x] 5.2 (P) Unit test wire framing
  - Framing round-trips correctly against a fixed, known byte sequence and against a local loopback connection
  - _Requirements: 2.1_
  - _Boundary: xcecommon.wire_
  - _Depends: 2.4_

- [x] 5.3 (P) Verify bundled schemas resolve under every install mode
  - Both the wire-protocol schema and the validation schemas resolve to real, parseable files under the currently active install
  - A deliberately corrupted or absent packaged schema produces the expected installation-hint error rather than a crash
  - Meaningful confirmation of the pinned install path requires running this under the release build profile
  - _Requirements: 5.5, 7.4_
  - _Boundary: xcecommon.schema, xcecommon.validation_
  - _Depends: 2.1, 2.2, 1.3_

- [x] 5.4 (P) Golden-file test config build determinism
  - Building from a minimal fixture, a full fixture covering all seven file types, and a set of intentionally invalid fixtures each produce the expected result
  - Building the same fixture twice produces byte-identical output and an identical checksum
  - _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 1.10, 1.11, 1.12, 9.1, 9.2, 9.3_
  - _Boundary: xceadmin.build_
  - _Depends: 3.1_

- [ ] 5.5 (P) Test every YAML validation failure mode
  - Each invalid-fixture case (schema violation, reserved name, duplicate name including a cross-type collision, undefined reference) produces the documented outcome — hard failure or warning, matching the requirement
  - _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5_
  - _Boundary: xcecommon.validation_
  - _Depends: 2.1_

- [ ] 5.6 (P) Test config push and dry-run against a mock controller
  - A mock TLS server standing in for the controller confirms the full push flow: successful push, redirect-following, and every documented error path
  - The same mock confirms dry-run reports both an accepted and a rejected outcome correctly
  - _Requirements: 2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, 3.1, 3.2, 3.3, 3.4, 6.1, 6.2, 6.3, 7.1, 7.2, 7.3_
  - _Boundary: xceadmin.push, xceadmin.discovery_
  - _Depends: 3.3_

## Implementation Notes

- **1.3**: nix's bare `python3` does not put `pip` on PATH — `python3Packages.pip` had to be added to `nativeBuildInputs` alongside the packages design.md's Technology Stack table lists; not a spec gap, just an operational dependency the table didn't need to spell out.
- **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()`.
- **2.3**: `map_all_sections()` (a `SECTION_MAP`-driven orchestrator, not explicitly named in design.md's Service Interface but consistent with its Contracts) is the single call `xceadmin.build` (task 3.1) needs — it treats a missing YAML file identically to a present-but-empty section (byte-for-byte), satisfying Requirement 1.5 structurally. `target.init(field, 0)` and never touching an empty/absent list field produce *different* serialized bytes for the same logical "nothing here" value (verified empirically) — the mapper always picks "never touch" for byte-determinism (9.1). Calling `.to_bytes()` twice on the same capnp builder triggers a spurious "message already written" `UserWarning` even when no field was mutated between calls — capture the bytes once and reuse them rather than calling `.to_bytes()` again.
- **2.4**: `errors.py`'s exception subclasses (task 1.2) all take `message` as an explicit first constructor argument in addition to their structured fields (e.g. `ConnectionTimeoutError(message, host, port)`, `ReplyDecodeError(message, reason)`) — design.md's abbreviated `Service Interface` type stubs (`class ConnectionTimeoutError(XceAdminError): host: str; port: int`) read as if only the structured fields were needed; always check the real constructor in `errors.py`, not the design doc's shorthand. `xcecommon.wire` is deliberately opcode-agnostic — `send_request`/`send_frame`/`recv_frame` move raw `bytes` only and never import a capnp type; callers own `.to_bytes()`/`.from_bytes()`. Reused `ha/data/xce.crt`/`xce.key` (the repo's existing dev cert pair, same one `deploy-ha.sh` already copies for controller TLS) as the test fixture for both the TLS test server and the client `cert_dir` — no synthetic cert generation needed.
- **3.1**: Requirement 1.7's "unrecognized-but-schema-valid field" warning is real and fires on essentially every real build using `src/configs/resources.yaml` — `resources.yaml`'s `type` key is *required* by `resources.schema.json` (`required: [name, type]`) but has no `ClusterConfig` destination (per `xcecommon.mapping`'s documented decision not to guess the `valtype`/`ordtype`/`flags` integer encoding), so every valid resource entry triggers a "field 'type' ... ignored" warning unconditionally, not just for unusual input. This is a disclosed spec tradeoff, not a bug — flagged here so a future pass over `xceadmin.build`'s warning UX (e.g. only warning once per field-name per run instead of once per item) knows the noise is expected, not a regression. `xceadmin.build`'s unmapped-field table (`_UNMAPPED_BUT_VALID_ITEM_KEYS`) was derived by cross-referencing `xcecommon.mapping`'s per-item builder functions against the real `src/configs/schema/*.schema.json` property lists directly (not from prose alone) to avoid drift.
- **3.2**: design.md's Dependencies list for `xceadmin.discovery` names only `xcecommon.wire`, but decoding a reply to inspect `RequestReply.redirectHost`/`.redirectPort` genuinely requires `xcecommon.schema` too (verified real field names against `controller.capnp:594-595`, which matched design.md exactly this time — unlike task 2.2's `Reply`/`RequestReply` struct-name slip). Treated as a design-doc omission, not license to hand-roll capnp decoding. `follow_redirects`'s `cert_dir` parameter is accepted for signature parity with design.md but unused internally — all I/O, including TLS setup, is delegated to the caller's `send_fn` closure, which is expected to already carry its own `cert_dir`. A connection timeout to any hop (including a non-initial one reached via redirect) raises `NoPrimaryError` naming that specific hop, not the original target.
- **bug-1461 (out-of-band, between 3.2 and 3.3)**: `controller.capnp` had a real, confirmed schema bug — a locally-declared `struct Data` (an `OPCode_get` query-result tagged union) shadowed Cap'n Proto's builtin `Data` blob type for every later `: Data` field in the file, miscompiling `UpgradeConfigRequest.sha256`/`.bytes` (and 5 other fields across `ReplConfigReply`/`ReplConfigPushRequest`/`ReplEnvelope`/`SnapshotTransfer`/`WalCatchupEntry`/`CtrlHelloRequest`, several HA-critical) as a struct-pointer type instead of a byte blob — blocking `push_config()` from constructing a valid request via pycapnp. Fixed at the root (renamed the local struct to `XceData`, `$C.mapname("xce_data_t")` unchanged so the C typedef is stable) across 20 files spanning controller/lib/scheduler/Go — see commit `e53c086` and `.wolf/buglog.json` bug-1461 for the full investigation and fix. This was NOT an admin-config-tool-boundary change; it's a pre-existing, cross-component schema defect that happened to block this task. `push_config()`'s real controller counterpart remains untested live — a *separate*, already-documented dispatch-gate defect (`ha_dispatch_gate()`/`cluster.c`, spec.json `blocked_on`) still blocks that independently of bug-1461.
- **3.3**: `probe_config()` does NOT go through `xceadmin.discovery.follow_redirects` — `ProbeConfigTagReply` is a direct struct with only `configTag`, no `redirectHost`/`.Port` fields exist on it at all, so there is no redirect signal for `follow_redirects` (which unconditionally decodes replies as `RequestReply`) to read; only `push_config()`'s `Request`/`RequestReply`-wrapped exchange supports redirects. `ProbeConfigTagReply` also carries no accept/reject signal — `DryRunResult.accepted` is a client-side inference (true on any successful round-trip), matching the real C reference client (`config_repl.c`'s `xce_probe_config_tag`, which also always sends an empty `name` and returns only a bare tag). pycapnp union field assignment for Void/Text-typed variants (e.g. `reply.data.nodata = None`, `reply.data.debugmsg = "..."`) must use direct assignment, not `.init(name)` — `.init()` without a size is only valid for struct/list/data-typed variants, confirmed via `KjException: init() without a size is only valid for struct and object fields`.
- **4.1**: `xcecommon.wire.connect()` deliberately lets `ConnectionRefusedError`/other bare `OSError`s propagate unwrapped (documented in `wire.py`'s own module docstring) — `xceadmin.cli` is the layer that catches `OSError` and formats it per Requirement 7.1, alongside its `except XceAdminError` dispatch surface; a task 4.1 implementer must not assume `XceAdminError` alone covers every network failure mode. click 8.3.1's `CliRunner` no longer accepts `mix_stderr=`; `result.stdout`/`.stderr` are separated by default, but `CliRunner`'s output capture decodes as text and is **lossy for arbitrary binary** (confirmed: `bytes(range(256))` does not round-trip through the captured `.output` string) — do not assert exact byte content of a `-o -` binary write through `CliRunner`; that's already covered at the `write_output()` unit level (task 3.1's `test_config_build.py`). A CLI test that omits `-o` falls through to the real default (`cluster.bin`, relative to CWD) — always pass an explicit `-o <tmp_path>` in tests that reach the write step, or the test will write a stray artifact into the actual repo working directory (caught via `git status` before commit, not by the test itself).
- **4.2**: `deploy-ha.sh` resolves both `xceadmin` (`$SCRIPT_DIR/../xceadmin/.venv/bin/xceadmin`) and the YAML configs source (`$SCRIPT_DIR/../configs`) relative to the script's own location (`$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)`), not the caller's CWD — matches the project's other relative-path scripts and keeps the deploy working regardless of where it's invoked from. `--build-config` is parsed out of `"$@"` before positional-arg assignment so it can appear anywhere in the argument list, not just a fixed position. Verified live against the real `xceadmin` build and `src/configs/`: missing `cluster.bin` auto-builds (8.4), an existing one with no flag is left byte-for-byte untouched — same mtime, same SHA (8.5), and `--build-config` against an existing `cluster.bin` forces a rebuild that reproduces byte-identical content (8.3, confirming 9.1's determinism guarantee holds through this path too).
- **5.1, 5.2**: Already fully satisfied by task 2.3's `test_mapping.py` and task 2.4's `test_wire.py` respectively — same requirement lists, same acceptance bullets (in-memory-only mapper coverage + the explicit two-field case for 5.1; a fixed known byte sequence + real loopback round-trips for 5.2). No new tests written; would have been pure duplication.
- **5.3**: Found and fixed a real gap while writing `test_schema_bundling.py` — `xcecommon.validation.validate_yaml()`'s `json.loads(schema_path.read_text())` call (loading the *bundled JSON validation schema itself*, not the operator's YAML) had no error handling, so a corrupted bundled JSON schema crashed with a raw `json.JSONDecodeError` instead of a clean `ValidationError` — the same "crash vs. clean error" class of bug task 2.2 already fixed for `controller.capnp`, just on the JSON-schema side, and not caught by task 2.1 since that task's tests only exercised operator-YAML failure modes, never a corrupted *schema* file. Fixed by wrapping it and raising `ValidationError(..., constraint="schema-corrupted")`, matching the existing missing-schema handling's pattern one function above it. **Gotcha for any future task touching `xcecommon`/`xceadmin` source**: editing a source file does NOT propagate to a running `.venv` until reinstalled — always re-run `xce-build.sh xceadmin` after a source edit before trusting a bare `.venv/bin/python -m pytest` result; a stale venv (especially one left in `release`/pinned mode by a prior profile run) will silently test the *old* code and produce a false failure (or false pass) that looks like a real bug.
- **5.4**: `tests/fixtures/{minimal,full}/configs/*.yaml` + checked-in `expected.bin` golden files added to `test_config_build.py` (not a new file — design.md's File Structure Plan lists the SAME requirement set for `test_config_build.py` under both 3.1 and 5.4). `full/configs/*.yaml` is a byte-for-byte copy of `src/configs/*.yaml`; its golden SHA (`860cfd4f59ef...`) independently matches the tag `deploy-ha.sh` produced in task 4.2's live verification against the same source files — an unplanned but welcome additional confirmation of Requirement 9's cross-context determinism guarantee. Golden-file tests catch representation drift (bytes change even though decoded content is still semantically correct) that purely structural/decode-based assertions elsewhere in the suite cannot detect; regenerate `expected.bin` deliberately (via `build_cluster_config()`, sanity-checked by decoding it back) whenever a real, intentional field-mapping change lands — never hand-edit the binary.