lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(resource-value-verification): per-name accept/reject verification in resupdate (task 2.2)

Browse source 
cmd_resupdate() now issues a follow-up xcecli_resval() query per submitted
(resource, worker) pair after the update RPC, since update_resval_list()'s
reply can't carry per-item status. Reports accepted/rejected per name, with
a bounded retry (5x100ms) against propagation races.

Live-confirmed: `resupdate -n mem -v 5 -n not-a-real-resource -v 1` reports
mem accepted and not-a-real-resource rejected in the same invocation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-18 00:24:44 +00:00
parent 058335d2a6
commit f5390c14e2
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
2 changed files with 123 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

3
.kiro/specs/resource-value-verification/tasks.md
View file

@ -23,7 +23,7 @@
- _Requirements: 1.1, 1.4_
- _Depends: 1.1_
- [ ] 2.2 Implement per-name accept/reject verification in resupdate
- [x] 2.2 Implement per-name accept/reject verification in resupdate
- After `cmd_resupdate()` sends the update RPC, issue one follow-up `xcecli_resval()` query per submitted `(resource, worker)` pair (the same client-library call `resval`/`cmd_resval` already 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`/`-v` pairs
@ -118,4 +118,5 @@
## Implementation Notes
- Task 1.1: `resval_typeresolve.c`'s new function is `int resval_resolve_type(const char* name, int type_override)`, returning a `Value_u_which` tag or `RESVAL_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`'s `set_resval()`/`cmd_resupdate()` were deliberately left untouched by task 1.1 — task 2.1 is the one that makes `set_resval()` a thin wrapper.
- `controller_unit_test` (the big legacy Unity binary) has a pre-existing, unrelated flaky failure in `test_config_repl.c` (bug-1561, overlayfs `fdatasync errno=5` sandbox artifact) that shows up on every `/xce-build controller` run 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 a `controller_unit_test` failure as real, and confirm via `git status` that `test_config_repl.c`/`config_repl.c` are untouched by the current task's diff.
- Task 2.2: `src/controller/meson.build` compiles `test.c` only into the `xce_test` binary (`xce_test_src`), never into `controller_unit_test` (`controller_unit_test_src`) — a `test.c`-only diff structurally cannot regress `controller_unit_test`'s results. Also: `controller_unit_test` is built with ASAN `detect_leaks=0` as pre-existing, repo-wide meson policy (not something any task in this feature turned off) — to actually check a `test.c` change for new leaks, run `xce_test` directly 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 `-t` apply to its preceding `-n`/`-v` pair) can silently drop an unrelated existing guard (here: duplicate-`-v` rejection) that depended on the old immediate-finalization timing. When touching `cmd_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_test` also needs `ASAN_OPTIONS=detect_leaks=0` for reliable non-interactive stdin-piped smoke testing (its default leak-detection exit path can swallow buffered stdout).

123
src/controller/testsuite/test.c
View file

@ -3364,12 +3364,116 @@ static int set_resval(
vals[idx].host = get_setting("myname");
return rc;
}
/*
* resource-value-verification task 2.2: the controller's own update RPC
* reply can never distinguish per-name accept/reject -- update_resval_list()
* (src/controller/worker.c:185-198) unconditionally returns success
* regardless of any per-item failure (design.md's "set_resval()/
* cmd_resupdate" component, Responsibilities & Constraints). To recover
* per-name accept/reject (Requirements 1.2, 1.3), cmd_resupdate() issues one
* follow-up xcecli_resval() query per submitted (resource, worker) pair --
* the exact client-library call resval/cmd_resval already use -- and
* compares the returned value against what was just sent.
*/
#define RESVAL_VERIFY_MAX_ATTEMPTS (5)
#define RESVAL_VERIFY_RETRY_MS (100L)
/* Compare a just-submitted value against a follow-up query's result. Both
* sides are xce_val_t (wire value union) -- a match requires the same
* `which` tag and an equal payload. */
static int resval_value_matches(xce_val_t* sent, xce_val_t* got)
{
if ((sent == NULL) || (got == NULL) || (sent->which != got->which)) {
return 0;
}
switch (sent->which) {
case Value_u_ivalue32:
return sent->u.ivalue32 == got->u.ivalue32;
case Value_u_uvalue32:
return sent->u.uvalue32 == got->u.uvalue32;
case Value_u_ivalue64:
return sent->u.ivalue64 == got->u.ivalue64;
case Value_u_uvalue64:
return sent->u.uvalue64 == got->u.uvalue64;
case Value_u_svalue:
return EQUAL_STRING(sent->u.svalue, got->u.svalue);
case Value_u_fvalue:
return float_equal(sent->u.fvalue, got->u.fvalue);
case Value_u_bvalue:
return (sent->u.bvalue == 0) == (got->u.bvalue == 0);
default:
return 0;
}
}
/*
* Query a just-submitted (name, host) pair back from the controller and
* print its per-name accepted/rejected verdict (Requirements 1.2, 1.3). A
* short bounded retry guards against a genuinely-registered resource whose
* value hasn't propagated to the query path yet (design.md's Implementation
* Notes: sub-second in practice for a controller-local query) -- NOT an
* arbitrary long poll.
*/
static void resupdate_report_one(char* name, char* host, xce_val_t* sent)
{
int attempt;
int ever_found = 0;
if (name == NULL) {
/* set_resval()'s Value_u_svalue fallback path leaves .name unset
* (pre-existing gap predating this task -- see set_resval()'s own
* comment above); nothing usable to query or report here. */
return;
}
for (attempt = 0; attempt < RESVAL_VERIFY_MAX_ATTEMPTS; attempt++) {
xce_resval_t** got = NULL;
int rc;
if (attempt > 0) {
usleep((useconds_t)(RESVAL_VERIFY_RETRY_MS * US_PER_MS));
}
rc = xcecli_resval(active_ctx->ctx, host, name, &got);
if (rc > 0) {
int matched = resval_value_matches(sent, got[0]->val);
int i;
ever_found = 1;
for (i = 0; i < rc; i++) {
free_ResourceVal_ptr(&got[i]);
}
FREEUP(got);
if (matched) {
printf("%s: accepted\n", name);
return;
}
/* Found but the value differs -- could still be a stale value
* about to be overwritten by the just-sent update's propagation,
* so keep retrying within the bounded budget rather than giving
* up immediately. */
continue;
}
}
if (ever_found) {
printf("%s: rejected: value mismatch\n", name);
} else {
printf("%s: rejected: unknown resource\n", name);
}
}
int cmd_resupdate(int argc, char* argv[])
{
#define MAX_N_RES (16)
int nres;
xce_resval_t vals[MAX_N_RES];
xce_resval_t* valptrs[MAX_N_RES];
char* subnames[MAX_N_RES];
char* resname = NULL;
char* resval_str = NULL;
int type_override = RESVAL_TYPE_UNKNOWN;
@ -3394,7 +3498,12 @@ int cmd_resupdate(int argc, char* argv[])
}
rc = set_resval(vals, nres, resname, resval_str, type_override);
if (rc >= 0) {
valptrs[nres] = &vals[nres];
valptrs[nres] = &vals[nres];
/* Tracked independently of vals[nres].name: set_resval()
* leaves .name unset on the Value_u_svalue fallback path
* (see its own comment), but `resname` here is always the
* exact name that was actually submitted for this pair. */
subnames[nres] = resname;
nres++;
}
resname = NULL;
@ -3436,14 +3545,24 @@ int cmd_resupdate(int argc, char* argv[])
if (resname != NULL && resval_str != NULL) {
rc = set_resval(vals, nres, resname, resval_str, type_override);
if (rc >= 0) {
valptrs[nres] = &vals[nres];
valptrs[nres] = &vals[nres];
subnames[nres] = resname;
nres++;
}
}
rc = xcecli_update_resvals(active_ctx->ctx, nres, &valptrs[0]);
if (rc >= 0) {
int i;
printf("resource update ok!\n");
/* The update RPC's own reply can't carry per-name accept/reject
* (see the comment above resupdate_report_one()) -- verify each
* submitted pair individually via a follow-up query instead. */
for (i = 0; i < nres; i++) {
resupdate_report_one(subnames[i], vals[i].host, vals[i].val);
}
} else {
SET_ERROR(-rc);
perror("xcecli_update_resval");