diff --git a/.kiro/specs/resource-value-verification/design.md b/.kiro/specs/resource-value-verification/design.md index 2e8aa206..7ae1aa3f 100644 --- a/.kiro/specs/resource-value-verification/design.md +++ b/.kiro/specs/resource-value-verification/design.md @@ -138,8 +138,8 @@ src/tools/ ```mermaid sequenceDiagram participant Skill as xce-test-resources - participant Worker as xce_test (worker ctx) - participant Client as xce_test (client ctx) + participant Worker as xce_test (worker ctx, ONE session) + participant Nested as xce_test (nested ! client ctx) participant Ctrl as controller participant Sched as scheduler daemon participant Api as apiserver @@ -152,9 +152,12 @@ sequenceDiagram Ctrl->>Api: notify schedUpdateResvals Sched->>Sched: apply + trace-log resource, host, value Api->>Api: store.ResourceLocations.Put + debug-log resource, host, value - Skill->>Client: resval -n -m synthetic-worker - Client->>Ctrl: query - Ctrl-->>Skill: controller-observed value + Worker->>Nested: ! --internal-controller-check (worker STILL connected) + Nested->>Ctrl: resval -n -m synthetic-worker (query) + Ctrl-->>Nested: controller-observed value + Nested-->>Worker: result written to a marker file (nested exit code ignored) + Worker->>Ctrl: quit (worker disconnects HERE -- not before) + Skill->>Skill: parse marker file for controller-observed value Skill->>Sched: tail/grep scheduler log for expected line (bounded wait) Sched-->>Skill: scheduler-observed value (or timeout) Skill->>Api: tail/grep apiserver log for expected line (bounded wait) @@ -162,6 +165,8 @@ sequenceDiagram Skill->>Skill: compare all three vs injected value, emit verdict ``` +**Load-bearing constraint the diagram makes explicit (as-built, bug-1579)**: the controller-layer check is NOT an independently-launched client session run after the worker disconnects. Any host-attrs-derived resource (`type`/`model`/`cpuf`/`maxcpus`/`maxmem`/`maxswap`/`maxtmp`/`ndisks` — `worker.c`'s `N_HOST_ATTRS`) is reset to baseline by the controller the instant the injecting worker disconnects — so a controller-layer check run as a separate, later session against any host-attrs name would ALWAYS false-MISMATCH, independent of whether the underlying propagation is actually correct (discovered as bug-1579, task 6.2, round-1 REJECTED). The fix nests the controller-layer `resval` query INSIDE the injecting worker's own still-open session, via `xce_test`'s `!` escape (`preprocess_and_run()` — works in any REPL context, including `worker`, with no `leave_ctx()`/disconnect) — the query genuinely runs before `quit`, not merely before some unrelated timeout. The scheduler and apiserver legs have no such constraint: both are pure log-tail/grep against a file, entirely independent of whether the worker connection is still open, so they run after the worker session ends like any other step. + Key simplification from the superseded design: no `record`/`replay`/REPL round-trip and no REST/`xcecli` query are needed — the scheduler and apiserver legs both poll their own live daemon's log file for the trace line the injection is expected to produce, within a bounded wait, exactly like the controller leg's own bounded-wait posture. This removes the REPL's `step`/`continue` pitfall entirely (there is no replay step in this flow) and the apiserver's auth/RBAC config from the critical path (see research.md Design Decisions). ## Requirements Traceability @@ -343,6 +348,7 @@ This feature is a verification tool, not a production service — "errors" are p - **Layer mismatch** (Requirements 3.2, 4.2, 5.2): the skill reports which layer(s) failed, with both the injected and observed value, per Requirement 6.3. - **Layer unreachable** (e.g. apiserver never came up, or the expected trace line never appears in the scheduler/apiserver log before the bounded wait elapses): treated the same as a mismatch for verdict purposes (FAIL, layer identified, "timeout" distinguished from "mismatch" in the report), not a silent skip. - **Deployment failure** (controller/scheduler/apiserver fails to start): the skill aborts before attempting injection, with a clear "deployment failed at step X" message. **Cleanup still runs on this path** — this is a deliberate strengthening of `xce-deploy-ha`'s precedent, not a mirror of it: `xce-deploy-ha`'s `deploy-ha.sh` has no trap-based cleanup and leaves already-started processes running on a mid-deploy failure, requiring the caller to run its separate `cleanup.sh` manually (confirmed against `src/tools/deploy-ha.sh`). Requirement 6.5 promises cleanup by default regardless of outcome, unless the operator explicitly requested the deployment be left running — a deployment failure is not that request, so the skill installs a trap (or equivalent) covering every deployment step from the first process start onward, not just the post-injection verification path. +- **Custom host-attrs resource name false-MISMATCH (known residual risk, not fully closed)**: the default resource set avoids every host-attrs-derived name (`type`/`model`/`cpuf`/`maxcpus`/`maxmem`/`maxswap`/`maxtmp`/`ndisks` — `worker.c`'s `N_HOST_ATTRS`) specifically to route around this; the System Flows section above documents the nested-`!`-escape fix and why it closes the controller-layer check for that default set. If an operator passes a CUSTOM `resource=value` argument naming a host-attrs resource, the controller-layer check will still report a false MISMATCH — not because propagation is wrong, but because that resource is reset to baseline the instant the worker session ends, and no code path today re-checks it before that reset for an arbitrary caller-supplied name. This is a known, documented gap (tasks.md task 6.2), not silently swallowed: a genuine fix needs either a C-level `xce_test` change (no REPL path from `worker` context to `client` context without an equivalent disconnect) or a bash coprocess restructuring to keep the worker connected across every layer's check — out of scope for the current implementation. The skill should warn (not silently mismatch) when a caller-supplied resource name matches a known host-attrs name. ### Monitoring Not applicable — this is an on-demand, operator-invoked CLI tool, not a running service. Logs are written under the run's scratch directory, following `xce-deploy-ha`/`xce-test-ha`'s existing `logs/` convention, for post-run inspection. diff --git a/src/frontend/apiserver/backend/handler.go b/src/frontend/apiserver/backend/handler.go index 694463c9..402afb62 100644 --- a/src/frontend/apiserver/backend/handler.go +++ b/src/frontend/apiserver/backend/handler.go @@ -99,6 +99,16 @@ func (h *DefaultHandler) OnResourceLocationUpdated(res *models.ResourceDistribut // field carrying whichever of StringValue/NumericValue/BooleanValue is // live for res.ValueType, so a downstream grep sees exactly what was // stored, not every possibly-zero field on the struct. + // + // DO NOT change the "resource-location stored" message string, or the + // resource/host/value field names below, without also updating + // query_apiserver_resvals() in + // .claude/skills/xce-test-resources/run-resource-verify.sh (design.md's + // own Revalidation Triggers list this exact pair) -- that script greps + // this exact message and parses zap's console-encoder trailing JSON + // block for these field names; an unrelated logging cleanup here + // silently turns every future verification run into a false + // UNREACHABLE/timeout, indistinguishable from a real regression. var valueField zap.Field switch res.ValueType { case int(models.STRING): diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index 6e9909c6..ad96dbcd 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -835,6 +835,16 @@ int xce_core_push_events(xce_sched_core_t* core, xce_core_event_t* events, * Debug/trace level only (tlog_debug3 via TRACE) -- no * control-flow change, fires on every applied delta. * + * DO NOT change the "resval applied host=... resource=... + * value=..." prefix (either branch below) without also + * updating query_scheduler_resvals() in + * .claude/skills/xce-test-resources/run-resource-verify.sh + * (design.md's own Revalidation Triggers list this exact + * pair) -- an unrelated logging cleanup that reflows this + * text silently turns every future verification run into a + * false UNREACHABLE/timeout, indistinguishable from a real + * propagation regression without investigation. + * * bug-1573 fix (round-2 remediation): Value_u_which has 7 * tags (ivalue32/uvalue32/ivalue64/uvalue64/svalue/fvalue/ * bvalue -- controller.capnp.h) and Value_u_bvalue is a real,