feat(xceadmin): validate percent-based limit values are in [0.0, 1.0] (bug-1833)

Adds an if/then JSON Schema (draft 2020-12) conditional constraint to
limits.schema.json's Limit definition: when percent is true, value
must be a number in [0.0, 1.0]. This is the class of authoring mistake
bug-1831/bug-1832 fixed manually (night_mem_pct: value: 50 instead of
0.5) -- now caught automatically at real `xceadmin config build` time
(the actual cluster.bin generation path deploy-ha.sh uses), not merely
by a unit test.

Empirically verified against the real jsonschema library this repo
uses: a violation surfaces with the exact same file/json_pointer/
constraint contract validate_yaml()'s existing ValidationError already
provides, so zero changes were needed to its error-reporting code.

While implementing this, the new constraint caught an identical
pre-existing defect in xceadmin's own checked-in "full" golden test
fixture (fixtures/full/configs/limits.yaml, night_mem_pct: value: 50)
-- fixed the same way, and regenerated the checked-in expected.bin
golden artifact to match the corrected input.

Added 3 tests to test_validation.py: reject an out-of-range percent
value, accept an in-range one, and confirm a non-percent value is
never range-checked regardless of magnitude.

Build gotcha discovered and documented (cerebrum.md): this repo has
two copies of every bundled schema -- src/configs/schema/ (canonical)
and src/xceadmin/src/xcecommon/schema_files/json/ (a gitignored local
mirror meson-build.sh always overwrites from the canonical copy on
every build). Only the canonical copy needed a source-controlled edit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-20 10:50:21 +00:00
parent a8fb768526
commit 7982f28557
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
6 changed files with 111 additions and 3 deletions

View file

@ -240,3 +240,4 @@
- **(2026-09-14, bug-1808) Unity's `TEST_ASSERT_*` macros abort a failing test via `longjmp()` (`TEST_PROTECT()`), which skips every line of code AFTER the failing assertion in that same function — including cleanup code placed only at the bottom of the function body (e.g. `clitest_kill_and_reap()`+`clitest_close()` in `test_test_cli.c`, tearing down a spawned pty-driven child process).** A single assertion failure anywhere mid-test therefore leaks that test's spawned child (left running forever, blocked on its own prompt) plus its pty/pipe fds — confirmed directly by finding 4 live orphaned `xce_test --norc` processes under one in-progress `harness_unit_test` run. Unity DOES guarantee `tearDown()` runs after every `RUN_TEST()` regardless of pass/fail/abort, so that (not each test's own inline "happy path" cleanup) is the only reliable place to catch this — but `harness_test.c`'s shared `tearDown()` (used by all 13 Unity test groups in that binary) was a no-op beyond a cosmetic print. **General rule: in any Unity (or other longjmp-based C test framework) suite, resource cleanup for a value that only exists on ONE test function's own stack must not rely on inline code after the last assertion — either wire it through the framework's guaranteed-to-run teardown hook, or track it in a process-wide static/global (not a stack pointer, which dangles once the longjmp unwinds past it) that the teardown hook can safely inspect and reap.** Fixed via a static `clitest_session_t` mirror updated by spawn/kill_and_reap/close, reaped from `harness_test.c`'s `tearDown()`; ASAN build found no sanitizer issues in the new code.
- **(2026-09-15, controller-status Req 12 discovery/requirements) Any new `ctrlstatus`-response field that describes a property of the current PRIMARY (not just the responding node itself) forces an explicit standby-fidelity design fork, and it is worth asking the user about it up front rather than assuming.** Options are: (a) propagate the value to standbys (learned via heartbeat, matching the precedent Req 10 already set for committedLsn/committedConfigVersion) so any reachable node answers accurately, or (b) report "unknown" on a standby even when it already knows the primary's name/term. Asked explicitly for the new "primary elected in term X at time Y" fields; user chose (a), full learned fidelity — consistent with how `primaryName` (Req 2.3) and committed position (Req 10) already work off-primary. Default to proposing (a) as the recommended option when this fork recurs, since it's the established pattern for this spec, but still confirm rather than assume, since it materially changes design/task scope (heartbeat payload changes) vs. a pure read of local state.
- **(2026-09-15) `controller-status`'s spec.json tracks its delta/reopen history in a single top-level `delta` object, not an array** — when starting a new delta cycle on top of one that already reached `validate_impl.decision: "GO"`, preserve the just-closed cycle by nesting it under `delta.prior_delta` (reopened_at/reason/new_requirements/impl_status) rather than overwriting it outright, so the requirement-number provenance ("which delta added Req N") stays recoverable from spec.json alone without needing git blame.
- **(2026-09-20, bug-1833) `xceadmin`'s bundled JSON schemas exist in TWO copies on disk, and editing the wrong one is silently discarded on the next build.** `src/configs/schema/*.schema.json` is the REAL canonical source; `src/xceadmin/src/xcecommon/schema_files/json/*.schema.json` is a checked-in mirror that `meson-build.sh:104` unconditionally overwrites from the canonical copy on every `xceadmin` build (`cp "$SRC_DIR/configs/schema/"*.schema.json "$SCHEMA_DST/json/"`). Editing only the xceadmin-tree copy produces a schema change that appears correct in `git diff` but has ZERO effect on any real build or test run — confirmed by editing only that copy, rebuilding, and watching a new schema constraint simply not fire (no error, just silent no-op), then finding the sync line and editing both copies to fix it. **Before editing any file under `src/xceadmin/src/xcecommon/schema_files/`, grep `meson-build.sh` for a sync/cp step first** — the same "editing xceadmin source doesn't retroactively update an already-built snapshot" caution documented elsewhere in this file extends to specific FILES within the tree being silently resynced from elsewhere, not just whole-package staleness.