run-resource-verify.sh deploys controller+scheduler(-vvv)+apiserver(debug logging) via xce-deploy-ha's deploy-ha.sh -> start-all.sh into a fresh scratch dir, confirms all three are genuinely up, and cleans up via a trap registered before any process starts - covering both normal completion and a mid-deployment abort. Logs always preserved regardless of --keep-running. Independently stress-tested with 8+ SIGTERM interrupts at varying deployment points and a genuine crashing-apiserver scenario - zero leftover processes in every case. Single-node by default (propagation testing, not HA failover), overridable via $XCE_RESVERIFY_PEERS. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
24 KiB
Implementation Plan
-
1. Foundation: test scaffolding and skill directory setup
-
1.1 Extract resupdate's type-resolution logic and write failing unit tests for its fallback behavior
- Move
set_resval()'s resource-name →Value_u_whichtable lookup into a new, small, dependency-lightresval_typeresolve.c/.hmodule with no networking/REPL/global-state dependency —test.c(4298 lines, its ownmain(), heavylibedit/network dependencies) cannot itself be white-box included into a unit test, so this extraction is a hard prerequisite for testing this logic in isolation at all - Create
test_resval_typeresolve.c(Unity) and wire both new files intosrc/controller/meson.build: addtestsuite/resval_typeresolve.ctoxce_test_src(line 57) soxce_teststill links, and add a newtest()target fortest_resval_typeresolve.cfollowing the existingcontroller_unit_test/sched_push_unit_testpattern (lines 86, 215, 222) — these are hand-maintained explicit lists, not globs, so the module and test will not build without this step - Write tests covering: a currently-known name (e.g.
mem) still resolves numeric exactly as today; a currently-known string name (type) still resolves string exactly as today; an unrecognized name (e.g.r15m) defaults to numeric; an unrecognized name with an explicit type-override flag resolves to the requested type - Observable completion: the new module and test target both compile and link via
meson, and the new test target fails (RED) because the fallback-to-numeric and type-override behavior don't exist yet - Requirements: 1.1, 1.4
- Move
-
1.2 (P) Scaffold the xce-test-resources skill directory
- Create the skill directory with a
SKILL.mdstub describing the intended usage (/xce-test-resources [resource=value ...]) and the VERDICT line format, following thexce-test-ha/xce-deploy-haSKILL.md convention - Observable completion: the skill is discoverable/invocable (even though its underlying script is not yet functional) and its documented contract matches design.md's Batch/Job Contract for the skill
- Requirements: 6.1
- Boundary: xce-test-resources skill
- Create the skill directory with a
-
2. Core: resupdate resource-name coverage fix
-
2.1 Wire the extracted resolver into resupdate and add the type-override flag
- Extend
resval_typeresolve.c's fallback path so any resource name not in the existing table defaults to numeric (Value_u_fvalue), whiletype/modeland all currently-supported names keep their exact existing behavior - Make
test.c'sset_resval()a thin wrapper delegating toresval_typeresolve.c(for type resolution) plus its own existing.hosttagging (unchanged, still needs REPL state) - Add an optional
-t <numeric|string>flag tocmd_resupdate(), applied per-n/-vpair - Observable completion: the unit tests from 1.1 pass (GREEN); running
resupdate -n r15m -v 1.5in the worker context (against a live controller) now succeeds where it previously silently failed - Requirements: 1.1, 1.4
- Depends: 1.1
- Extend
-
2.2 Implement per-name accept/reject verification in resupdate
- After
cmd_resupdate()sends the update RPC, issue one follow-upxcecli_resval()query per submitted(resource, worker)pair (the same client-library callresval/cmd_resvalalready uses) and compare the returned value against what was just sent - Report "accepted" for a matching value, "rejected: unknown resource" for a not-found result, with a short bounded retry before concluding rejection (guards against a genuinely-registered resource whose value hasn't propagated to the query path yet)
- Report every submitted name's individual accept/reject outcome when a single invocation carries multiple
-n/-vpairs - Observable completion: against a live controller,
resupdate -n mem -v 5 -n not-a-real-resource -v 1reportsmemas accepted andnot-a-real-resourceas rejected, in the same invocation - Requirements: 1.2, 1.3
- Depends: 2.1
- After
-
3. Core: apiserver local deployment capability
-
3.1 (P) Extend
xce-deploy-ha'sdeploy-ha.shwith a conditional apiserver step and a combinedstart-all.sh- In
deploy-ha.shStep 3, add a new conditional branch in the same opt-in-by-presence spirit as the existing worker/scheduler branch: check for a pre-built apiserver binary at its own Go-build output path (e.g./tmp/apiserver,make build-server's default) — NOT alongside$BINARY, since apiserver is a separate build system, not the same meson build dir worker/scheduler share;deploy-ha.shdoes not invokemake build-serveritself, building it remains a caller prerequisite (document in SKILL.md's Prerequisites section, same pattern as the existing/xce-build controllernote). If present, generate$TOP/scripts/start-apiserver.shplus a minimallogger.yaml(modeled onmake test-integration'sXCE_BIN/SRV_URLconvention) — functional auth/RBAC is NOT required (verification is trace-log-based, task 6.4; no REST query is ever issued against this instance), butlogger.yamlMUST setlogging.level: debugor task 4.2's trace line is silently never written; absent the binary, generate nothing, so existing callers are unaffected - Also in Step 3, always generate
$TOP/scripts/start-all.sh: runsstart.shfor the controller node(s), waits on Step 5's existing health-check, then runs whichever ofstart-worker.sh/start-scheduler.sh/start-apiserver.shwere actually generated for this deploy (skipping any that weren't) — start the scheduler with-vvvspecifically (verbosity 3, matchingtlog_debug3/TRACE), overriding SKILL.md's existing-v-only example for this feature's needs, or task 4.1's trace line is silently never written - Update
.claude/skills/xce-deploy-ha/SKILL.mdwith a new "Step 4d" documenting the apiserver step (following the existing Step 4b pattern) and documentingstart-all.shas a combined shortcut for Steps 4/4b/4c - Write apiserver/scheduler logs under the caller's scratch directory, matching the existing
logs/convention — these are the logs tasks 6.3/6.4 grep - Observable completion: running
deploy-ha.shagainst a build that includes an apiserver binary, thenstart-all.sh, leaves a running controller, scheduler (at-vvv), and apiserver (with debug-level logging) with no further manual steps; runningdeploy-ha.shagainst a build without an apiserver binary (e.g.xce-test-ha's existing usage) is unaffected — nostart-apiserver.shis generated, andstart-all.shskips that step - Requirements: 5.3, 6.1
- Boundary: xce-deploy-ha (deploy-ha.sh, SKILL.md)
- In
-
4. Core: scheduler and apiserver trace-log instrumentation
-
4.1 (P) Add a trace log line to the scheduler's live resource-update handling
- In
xce_core_push_events()'sXCE_CORE_EVENT_RESVALS_DELTAcase (src/scheduler/src/core_events.c), add oneTRACE-level log line (this component's existingtinylogconvention) immediately after the resolved worker's value is written, logging the resolved host, resource name, and applied value in a fixed, greppable format - Confirm the existing scheduler testsuite (
test_core_events.cand friends) still passes unchanged — this is a pure log addition with no control-flow change, so no new unit test is required (consistent with this codebase's existing precedent: otherTRACE/WARNcalls in this area, e.g.apiserver_worker_change, have no dedicated unit test coverage either); the log line's actual content/format is confirmed live in task 8.1 - Observable completion: a live scheduler daemon's log file contains one line per applied resource update, in the format task 6.3 will grep for; existing scheduler tests are unaffected
- Requirements: 4.1, 4.2, 4.3
- In
-
4.2 (P) Add a trace log line to the apiserver's resource-update storage handling
- In
OnResourceLocationUpdated(src/frontend/apiserver/backend/handler.go:92-95), add onelogger.Debug(...)call (zap, per this component's existing convention) immediately afterstore.ResourceLocations.Put(key, *res), logging the resource name, location/host, and stored value - Confirm existing apiserver backend tests still pass unchanged — same rationale as 4.1, no new unit test required for a pure log addition; content/format confirmed live in task 8.1
- Observable completion: a live apiserver's log file contains one line per stored resource-location update, in the format task 6.4 will grep for; existing backend tests are unaffected
- Requirements: 5.1, 5.2, 5.4
- In
-
5. Integration: full-stack deployment orchestration
-
5.1 Wire controller, scheduler, and apiserver deployment into the skill via
start-all.sh- Invoke
xce-deploy-ha'sdeploy-ha.sh(task 3.1) into a fresh scratch directory, then its generatedstart-all.sh— no manual per-component sequencing lives in this skill;start-all.shalready gates scheduler/apiserver startup on the controller health-check and handles the-vvv/debug-level logging needed for tasks 4.1/4.2's trace lines - Register a
trap-based cleanup (or equivalent) before invokingstart-all.sh, not just at the end of the script — this must cover a mid-deployment abort (e.g. apiserver fails to start after controller+scheduler are already up), not only a completed PASS/FAIL run; deliberately stronger thanxce-deploy-ha's own convention (a separate, manually-invokedcleanup.sh) because Requirement 6.5 requires cleanup-by-default on every path unless the operator passed the "keep running" flag - Observable completion: a single invocation of the orchestration script leaves a running controller, live scheduler daemon (built with task 4.1's trace log, started at
-vvv), and apiserver (built with task 4.2's trace log, debug-level logging enabled), all logging under the same scratch directory, with no manual steps; killing the script mid-deployment (e.g. before the apiserver step completes) still removes every process it had already started, unless "keep running" was passed - Requirements: 5.3, 6.1, 6.5
- Depends: 1.2, 3.1, 4.1, 4.2
- Invoke
-
6. Core: resource injection and per-layer verification checks
-
6.1 Implement worker join and resource injection
- Join a synthetic worker through the
xce_testworker context (no real worker daemon process), then runresupdatefor one or more resource names/values - Observable completion: after this step runs,
resupdatehas been accepted for every requested resource name with zero real worker daemon processes running - Requirements: 2.1, 2.2, 6.1, 6.4
- Depends: 2.1, 2.2, 5.1
- Join a synthetic worker through the
-
6.2 Implement the controller-side verification check
- Query the controller via
resvalfor each injected resource/worker pair and compare against the injected value - Observable completion: the check reports, per resource, whether the controller's observed value matches the injected value, including both values when it does not
- Requirements: 3.1, 3.2
- Depends: 6.1
- Boundary: xce-test-resources skill (controller check) — same file as 6.3/6.4 (
run-resource-verify.sh), sequential, not parallel-safe
- Query the controller via
-
6.3 Implement the scheduler-side verification check via trace-log tail
- Tail/grep the live scheduler daemon's log file (task 4.1's trace line) for each injected resource/worker pair, polling with a short interval up to a generous, explicit bounded wait — no record, no replay, no REPL
- Compare the scheduler's observed value against the injected value, reported distinctly from the controller-side result; a timeout with no matching line is reported as "unreachable," distinct from a value mismatch
- Observable completion: the check reports, per resource, whether the scheduler's observed value matches the injected value within the bounded wait, attributable specifically to the scheduler layer
- Requirements: 4.1, 4.2, 4.3
- Depends: 6.1, 6.2
- Boundary: xce-test-resources skill (scheduler check) — same file as 6.2/6.4, sequential
-
6.4 Implement the apiserver-side verification check via trace-log tail
- Tail/grep the live apiserver's log file (task 4.2's trace line) for each injected resource/worker pair, polling with a short interval up to a generous, explicit bounded wait — no
xcecliquery, no REST call - Compare the apiserver's observed value against the injected value, reported distinctly from the controller-side and scheduler-side results; a timeout with no matching line is reported as "unreachable," distinct from a value mismatch
- Observable completion: the check reports, per resource, whether the apiserver's observed value matches the injected value within the bounded wait, attributable specifically to the apiserver layer
- Requirements: 5.1, 5.2, 5.4
- Depends: 6.1, 6.3
- Boundary: xce-test-resources skill (apiserver check) — same file as 6.2/6.3, sequential
- Tail/grep the live apiserver's log file (task 4.2's trace line) for each injected resource/worker pair, polling with a short interval up to a generous, explicit bounded wait — no
-
7. Integration: verdict aggregation, reporting, and cleanup
-
7.1 Aggregate layer results, emit the verdict, and clean up
- Combine the controller/scheduler/apiserver check results into a single per-resource, per-layer comparison, implemented as a self-contained comparison step that accepts a set of (resource, injected value, per-layer observed value) records — so it can be invoked directly with synthetic records for testing (task 8.2), not only reachable from a live run
- Emit a machine-parseable VERDICT line (following
xce-test-ha's convention) plus the standard terminal PASS/FAIL oracle line, naming every layer that mismatched or was unreachable (timed out) - Support checking multiple resource names in one run, defaulting to the resource names added in task 2.1 when none are supplied
- Clean up every process and runtime/data-directory artifact this run created, unless the operator passed a flag requesting the deployment be left running — this is the normal-completion firing of the same trap-based cleanup task 5.1 registered for the abort path, not a separate mechanism. Log files under the scratch directory are always preserved, regardless of the keep-running flag, so a FAIL verdict's underlying scheduler/apiserver trace-log evidence remains inspectable after the run
- Observable completion: a full run against the default resource set produces exactly one VERDICT line and a matching process exit code, no controller/scheduler/apiserver processes remain afterward (unless the keep-running flag was used), and the run's log files are still present on disk regardless
- Requirements: 6.2, 6.3, 6.4, 6.5
- Depends: 6.2, 6.3, 6.4
-
8. Validation: end-to-end live verification runs
-
8.1 Run the full flow live, including one deliberately-unregistered resource name
- Execute the skill end-to-end against a freshly deployed controller/scheduler/apiserver, using the default resource set (task 2.1's previously-missing names) plus one resource name that does not exist in the cluster's registered set
- Observable completion: every genuinely-registered resource individually reports matching values at all three layers (confirming tasks 4.1/4.2's trace-log lines are correctly formatted and greppable, not just present), AND the unregistered name is reported as a clear, specifically-identified rejection (not a silent omission) with correct per-name accept/reject accounting across the whole batch; per design.md's Error Handling (an unrecognized name is a run-level failure), the run's single overall VERDICT is FAIL, attributable specifically to the unregistered name — not a false overall PASS
- Requirements: 1.1, 1.2, 1.3, 1.4, 2.1, 2.2, 3.1, 3.2, 4.1, 4.2, 4.3, 5.1, 5.2, 5.3, 5.4, 6.1, 6.2, 6.4
- Depends: 7.1
-
8.2 Verify the verdict logic correctly reports FAIL on a genuine mismatch
- Invoke task 7.1's comparison step directly with a synthetic set of per-layer observed-value records where one layer's value deliberately does not match the corresponding injected value (no live deployment required for this check, since the comparison step is self-contained per task 7.1)
- Observable completion: the comparison step reports an overall FAIL verdict that correctly names the specific layer whose synthetic record was mismatched, and reports PASS for every other, matching layer/resource in the same input set — proving the verdict logic distinguishes pass from fail rather than always reporting PASS
- Requirements: 6.3
- Depends: 7.1
Implementation Notes
- Task 5.1:
run-resource-verify.shexists now (.claude/skills/xce-test-resources/), structured with explicit "Extension point" comments marking where task 6.1 (injection), 6.2-6.4 (per-layer checks), and 7.1 (verdict/cleanup-on-every-path) hook in — read those before starting 6.1. Trap is registered as the first statement inmain(), before any process starts;cleanup()uses a single combinedpkill -9 -f(alternation pattern) plus retry-until-pgrep-clean, not sequential per-process kills (a sequential version was tried first and found non-deterministically flaky under an async SIGTERM trap — bug-1575). Default deployment is single-node (not 3-node HA) — deliberate, since this skill verifies propagation not failover; override via$XCE_RESVERIFY_PEERS. Known non-blocking gap:$XCE_RESVERIFY_SCHED_BIN/$XCE_RESVERIFY_APISERVER_BINonly gate this script's own pre-flight check, NOT which binarydeploy-ha.shactually deploys (it hardcodes/tmp/apiserverand derives the scheduler path from$BINARY's build dir) — don't rely on these two env vars to test against an alternate binary. - Task 4.2: the apiserver's trace log line (task 6.4 must grep for exactly this) is zap message
"resource-location stored"with fieldsresource(string),host(string),value(float64 for numeric resources — the common case; string fortype/model; bool for boolean-typed resources), e.g.{"resource": "r15m", "host": "review-worker", "value": 1.75}. Grep the log forresource-location stored, then parse"resource": "<name>"/"host": "<host>"/"value": <val>from the trailing field block. - Task 4.2 discovered bug-1574, which will affect task 8.1 and possibly task 7.1's default resource set — read before implementing either:
xce_testcan SEGFAULT onresupdate -n type -v .../resupdate -n model -v ...(the twoValue_u_svalue/string-typed resources). Root cause:set_resval()'s switch has noValue_u_svaluecase, sovals[idx].nameis left pointing at whatever garbage was already oncmd_resupdate()'s never-zero-initialized stack array — reading genuinely uninitialized stack memory, invisible to ASan (would need MemorySanitizer), and NON-DETERMINISTIC (task 2.2's reviewer saw-n type -v fooNOT crash under full ASAN; task 4.2's implementer saw-n model -v CustomModelXcrash immediately — different argv/stack layout, same underlying UB). Task 1.2'sSKILL.mdscaffold already documentstypeas part of an illustrative default resource set "for string-path regression coverage" — before task 7.1 finalizes the actual default resource set, either excludetype/modelfrom any default until bug-1574 is fixed, or fix bug-1574 first (it's a small, well-understood fix: zero-initializevals[]incmd_resupdate()and/or add the missingValue_u_svaluecase toset_resval()'s value-population switch — out of boundary for tasks 4.1-4.2, in boundary for whichever task next touchestest.c'scmd_resupdate()/set_resval(), most likely as a prerequisite fix before task 8.1's live e2e run). A crash mid-run would make task 8.1's or the skill's own live verification flaky/non-deterministic, undermining the whole feature's purpose. - Consolidated list of pre-existing (not-fixed-here) bugs surfaced by this feature's own live testing so far: bug-1571 (apiserver
RegisterToBackendgate checksAddressbeforePeers— worked around in generated config, not source, by task 3.1), bug-1572 (scheduler double-free/crash on a resource's 2nd periodic report — task 4.1), bug-1573 (see buglog.json, task 4.1), bug-1574 (xce_testsegfault on string-typedresupdate— task 4.2, described above). - Task 4.1: the scheduler's trace log line format (task 6.3 must grep for exactly this) is
resval applied host=<H> resource=<R> value=<V>atDEBUG3level (requires-vvv, per task 3.1'sstart-all.sh). Numeric resources use%f(fixed 6-decimal, e.g.12345.500000— parse-and-compare as float, not exact string match); string-typed resources (type/model) use%s. Any otherValue_u_whichtag (e.g.Value_u_bvalue, boolean resources) is silently NOT logged by this line (matches this file's own sibling-function convention) — not relevant to this feature's resource set, which is entirely fvalue/svalue. - Task 4.1 (round 2) discovered and logged, but deliberately did NOT fix (out of boundary, confirmed pre-existing since 2026-08-03/04, unrelated to this feature): bug-1572 — the live scheduler daemon double-frees and silently crashes on a worker's SECOND periodic resource report for the same resource (
core_events.c's RESVALS_DELTA case storesrv->valintow->resvalswithoutMOVE_PTR, whiledriver.c'srelease_translated_payload()unconditionally frees the same list). This feature's own resupdate-based injection flow (tasks 5.1+) should avoid resupdate-ing the exact same resource name twice against the same live scheduler within one run until bug-1572 is fixed separately, or the live e2e task (8.1) could intermittently crash the scheduler mid-run. - Task 1.1:
resval_typeresolve.c's new function isint resval_resolve_type(const char* name, int type_override), returning aValue_u_whichtag orRESVAL_TYPE_UNKNOWN(-1, doesn't collide with the real 0-6 enum range). Task 2.1 should build on this exact signature rather than inventing a new one.test.c'sset_resval()/cmd_resupdate()were deliberately left untouched by task 1.1 — task 2.1 is the one that makesset_resval()a thin wrapper. controller_unit_test(the big legacy Unity binary) has a pre-existing, unrelated flaky failure intest_config_repl.c(bug-1561, overlayfsfdatasync errno=5sandbox artifact) that shows up on every/xce-build controllerrun in this environment. It is NOT caused by this feature's changes — do not chase it in later tasks; cross-check.wolf/buglog.json's bug-1561 entry before treating acontroller_unit_testfailure as real, and confirm viagit statusthattest_config_repl.c/config_repl.care untouched by the current task's diff.- Task 3.1:
src/frontend/Makefile'sbuild-servertarget does not work standalone in this environment — it never setsCGO_CFLAGS/CGO_LDFLAGSthe waytest-backenddoes, failing withfatal error: client.h: No such file or directory. Pre-existing, out-of-boundary for this feature (confirmed unrelated to any of this feature's changes). To build a real apiserver binary for a live smoke test, export the same CGO flagsmake test-backendderives before runninggo build. Worth a separate bug-log entry / follow-up fix to the Makefile itself, not fixed here. Also found and worked around (config-only, no source change):main.go'sRegisterToBackendgate checksBackendYamlConfig.Addressbefore consultingPeers, so a generated config needsaddress/portpopulated from the first--peerscandidate, not justpeersitself (bug-1571). - Task 2.2:
src/controller/meson.buildcompilestest.conly into thexce_testbinary (xce_test_src), never intocontroller_unit_test(controller_unit_test_src) — atest.c-only diff structurally cannot regresscontroller_unit_test's results. Also:controller_unit_testis built with ASANdetect_leaks=0as pre-existing, repo-wide meson policy (not something any task in this feature turned off) — to actually check atest.cchange for new leaks, runxce_testdirectly with leak detection left ON, don't rely on the meson suite's leak-detection status. - Task 2.1 (round 1 REJECTED, round 2 fixed): refactoring a getopt-loop's flush/finalization timing (needed to let a trailing flag like
-tapply to its preceding-n/-vpair) can silently drop an unrelated existing guard (here: duplicate--vrejection) that depended on the old immediate-finalization timing. When touchingcmd_resupdate()-shaped CLI parsers in this codebase again, explicitly re-trace EVERY branch of the old getopt loop against the new one, not just the branches directly related to the new flag — a "traced every path" claim is only as good as the list of paths actually enumerated.xce_testalso needsASAN_OPTIONS=detect_leaks=0for reliable non-interactive stdin-piped smoke testing (its default leak-detection exit path can swallow buffered stdout).