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

docs(resource-value-verification): commit spec files, mark implementation complete

Browse source 
requirements.md, design.md, research.md, and spec.json were never committed
during implementation (only tasks.md was, per the per-task checkpoint
convention). Committing them now that all 15 tasks are implemented,
reviewed, and validated GO by /kiro-validate-impl.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-18 14:37:00 +00:00
parent 9be9e954c7
commit ef404131c0
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
4 changed files with 551 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

354
.kiro/specs/resource-value-verification/design.md Normal file
View file

@ -0,0 +1,354 @@
# Design Document
## Overview
This feature closes a coverage gap in `xce_test`'s worker-context `resupdate` command (its resource-name table is missing 7 real dynamic resources) and adds a repeatable, self-contained skill that injects a synthetic resource value through `resupdate` and confirms it is correctly received and stored at the controller, the live scheduler, and the Go apiserver — the exact three-layer propagation path that took a multi-hour live-deployment investigation to verify manually during bug-1569.
**Purpose**: Turn a class of bug (resource-value routing/propagation defects) that previously required deploying real worker/controller/scheduler/apiserver daemons and waiting on periodic reporting cycles into a command that reproduces and confirms the fix in seconds.
**Users**: XCE engineers debugging or regression-testing resource-value propagation across the controller/scheduler/apiserver boundary.
**Impact**: Extends `xce_test` (test.c) with a small, backward-compatible change; extends `xce-deploy-ha`'s `deploy-ha.sh`/`SKILL.md` with an opt-in, conditionally-generated apiserver deployment step (see Design Decisions below — no change to existing callers that don't need an apiserver); adds two single-line trace-log additions (scheduler, apiserver); adds one new skill. No production runtime *behavior* changes — the trace-log additions are inert log-only output at debug/trace level, and the `xce-deploy-ha` extension only activates when the apiserver binary is present, mirroring its existing worker/scheduler conditional-generation pattern.
### Goals
- `resupdate` accepts any resource name the cluster has registered (Requirement 1).
- Confirm the existing `join` + `resupdate` worker-context flow needs no real worker daemon (Requirement 2).
- A single command deploys controller + scheduler + apiserver, injects one or more resource values, and reports PASS/FAIL per layer (Requirements 3–6).
### Non-Goals
- Renaming `xce_test`, per-command help, reference documentation, JSON output mode, non-interactive batch mode, argument-convention unification — explicitly deferred to a future "enhance xce_test" project (see requirements.md Future Considerations).
- A new live scheduler admin query RPC — rejected in favor of trace-log observation, which needs no new RPC at all (see research.md Design Decisions).
- The record+replay+REPL flow and the apiserver REST/`xcecli` query path — both considered and superseded during design validation in favor of live trace-log observation at each layer's point of application; see research.md Design Decisions for why (this feature's own originating bugs, bug-1568 and bug-1569A, were both storage-layer defects found via exactly this technique, and the REPL's step/continue requirement had already produced one false negative live).
- General-purpose apiserver production deployment/HA — the new `xce-deploy-ha` apiserver step is a minimal single-node instance for local verification only, and no longer needs functional auth/RBAC since no REST query is issued against it.
- A new, standalone apiserver deploy script — considered and superseded in favor of extending `xce-deploy-ha` itself, reusing its already-precedented conditional (binary-present-only) generation pattern instead of re-implementing the same conventions in parallel (see research.md Design Decisions).
- CI/automated-pipeline integration.
- Fixing unrelated bugs the verification run happens to surface — those are reported, not silently patched, by this feature.
## Boundary Commitments
### This Spec Owns
- `set_resval()`/`cmd_resupdate` in `src/controller/testsuite/test.c`: accepting any registered resource name, with an explicit type-override escape hatch.
- A single trace/debug log line in `xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` case (`src/scheduler/src/core_events.c`), emitted at the point the live scheduler daemon applies a resource update — the only scheduler-side change this spec makes.
- A single trace/debug log line in `OnResourceLocationUpdated` (`src/frontend/apiserver/backend/handler.go:92-95`), emitted at the point the apiserver stores a received resource update — the only apiserver-side change this spec makes.
- An opt-in, conditionally-generated apiserver deployment step added to `xce-deploy-ha`'s `src/tools/deploy-ha.sh` and documented in its `SKILL.md` (Step 4d, alongside the existing Step 4b worker/scheduler steps) — generates `start-apiserver.sh` only when a pre-built apiserver binary already exists at its own Go-build output path (same "caller opts in by having already built it" spirit as the worker/scheduler conditional, though checked at a different path since apiserver is a separate build system — see Components for the precise distinction). Existing `xce-deploy-ha` callers that never build an apiserver (e.g. `xce-test-ha`) see zero behavior change.
- A new skill (`.claude/skills/xce-test-resources/`) that orchestrates deploy + inject + three-layer verify + PASS/FAIL report.
### Out of Boundary
- `resource_sync.c`'s real, production periodic load-index collection — unchanged.
- The controller's resval-forwarding hooks (`do_update_resvals`, `sched_resource_hook_fun`, `apiserver_resource_hook_fun`) — unchanged; this design only consumes them.
- The scheduler's REPL (`repl.c`/`repl_info.c`), recording admin RPCs, and the apiserver's REST API/`xcecli` query path — none of these are touched or driven by this design (superseded by the trace-log decision; see research.md Design Decisions). They remain available as a future follow-up if a query-layer-specific check is ever needed.
- All other scheduler/apiserver logic beyond the two single-line trace additions listed under "This Spec Owns" above — no behavior change, no new code paths, purely additive log output.
- `xce-deploy-ha`'s existing controller/worker/scheduler steps (1–4c) — unmodified; only a new, conditionally-triggered step is added alongside them.
- The broader "enhance xce_test" ideas listed in requirements.md's Future Considerations.
### Allowed Dependencies
- `xce-deploy-ha`'s controller deployment conventions and its existing worker/scheduler conditional-generation pattern (directory layout, TLS cert generation, `cluster.bin` build via `xceadmin`, Step 3's "generate the extra start script only if the binary exists" rule) — reused directly, by extending the same script, not duplicated in a parallel one (see research.md Design Decisions).
- `xce_test`'s existing worker context (`join`, `resupdate`) and client context (`resval`) — unmodified except for Requirement 1's table extension.
- `make build-server` (`src/frontend/Makefile`) to build the apiserver binary; `make test-integration`'s `XCE_BIN`/`SRV_URL` convention as the closest existing precedent for pointing it at a controller.
- Each daemon's own existing logging infrastructure (`tinylog` in C for the scheduler, zap `logger.Debug` in Go for the apiserver — per `error-handling.md`) as the channel the new trace lines are written through; no new logging mechanism is introduced.
### Revalidation Triggers
- The wire shape of `xce_resval_t` / `resupdate`'s CLI flags changes.
- `xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` handling, or `OnResourceLocationUpdated`'s call site, is refactored such that the trace log's format/location moves.
- `xce-deploy-ha`'s Step 3 conditional-generation pattern (or its directory layout) changes shape — this feature's apiserver step is now part of that same script, so a shape change there is a direct, first-party revalidation trigger rather than a drift risk between two separate scripts.
- If a future bug turns out to live specifically in the query/REST/serialization path rather than storage, revisit adding a real query-based check for that layer (not precluded by this design, just not built here).
## Architecture
### Existing Architecture Analysis
A resource value reaches all three layers through **one** controller-side write, not three independent paths (see research.md): `do_update_resvals()` applies it to the controller's own `w->host.resvals` (queried via `resval`) and fires `HOOK_DETAIL_RESOURCE_UPDATE`, which fans out to both `sched_resource_hook_fun` (scheduler) and `apiserver_resource_hook_fun` (apiserver). This design's job is purely to exercise this existing path with a synthetic worker and observe the result at each of the three existing points of application — it introduces no new production data path, and (per the trace-log decision) reads state at the storage point rather than through a query surface.
### Architecture Pattern & Boundary Map
```mermaid
graph TB
Skill[xce-test-resources skill]
WorkerCtx[xce_test worker context]
ClientCtx[xce_test client context]
Controller[xce_controller]
Scheduler[xce_scheduler daemon]
SchedLog[scheduler log]
Apiserver[apiserver]
ApiLog[apiserver log]
Skill --> WorkerCtx
WorkerCtx -->|join and resupdate| Controller
Skill --> ClientCtx
ClientCtx -->|resval query| Controller
Controller -->|resource update notify| Scheduler
Controller -->|resource update push| Apiserver
Scheduler -->|trace log on apply| SchedLog
Apiserver -->|trace log on apply| ApiLog
Skill -->|tail and grep| SchedLog
Skill -->|tail and grep| ApiLog
```
**Architecture Integration**:
- Selected pattern: black-box orchestration over existing interfaces plus two minimal, inert trace-log additions at each layer's point of application — this feature adds a coordinating layer (the skill), one small compatibility fix (`resupdate`'s resource table), and the two log lines; it does not add a new data path or query surface.
- Domain boundaries: `resupdate` table fix (C, test.c) is independent of the skill (bash orchestration); the `xce-deploy-ha` apiserver extension is independent of both and reusable by any `xce-deploy-ha` caller, not just this skill; the two trace-log additions are independent single-line changes in their respective daemons.
- Existing patterns preserved: `xce-deploy-ha`'s scratch-directory deploy convention and worker/scheduler conditional-generation pattern (extended, not duplicated, for apiserver), `xce-test-ha`'s VERDICT-line reporting convention, each daemon's own existing logging infrastructure (no new logging mechanism).
- New components rationale: the `xce-test-resources` skill is new because no existing tool exercises all three layers together; the `resupdate` fix is new because the current table is demonstrably incomplete; the `xce-deploy-ha` apiserver step and `start-all.sh` extend an existing tool rather than introduce a new one, following its own established conditional-generation precedent; the two trace-log lines are new because neither layer currently logs the value it applies (confirmed by direct inspection — see research.md).
- Steering compliance: production code changes are two single-line, debug/trace-level log additions with no behavior change, plus `test.c`'s test-tooling scope; TDD applies to the `set_resval()` change (see Testing Strategy).
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|------------------|-------|
| Test client (C, gnu17) | `xce_test` (existing binary, `src/controller/testsuite/test.c`) | Inject via worker context; query controller via client context | Requirements 1, 2, 3 |
| Scheduler (C, gnu17) | Live `xce_scheduler` daemon (existing, one new trace-log line) | Applies the resource update and logs it at the point of application | Requirement 4; see research.md Design Decisions |
| API server (Go) | `apiserver` binary via `make build-server` (one new trace-log line in `backend/handler.go`) | Receives and stores the resource update, logs it at the point of application | Requirement 5; see research.md Design Decisions |
| Orchestration | Bash skill script, `xce-deploy-ha`/`xce-test-ha` conventions | Deploy, inject, tail/grep each layer's log, verify, report, clean up | Requirement 6 |
## File Structure Plan
### Directory Structure
```
src/
└── controller/testsuite/
├── resval_typeresolve.c # NEW: set_resval()'s name→Value_u_which resolution, extracted for isolated unit testing
├── resval_typeresolve.h # NEW: its declaration
├── test_resval_typeresolve.c # NEW: Unity unit tests for resval_typeresolve.c (4 cases, see Testing Strategy)
└── test.c # MODIFIED: set_resval() delegates to resval_typeresolve.c; cmd_resupdate() gains -t flag + post-update accept/reject verification
src/controller/meson.build # MODIFIED: add resval_typeresolve.c to xce_test_src (both xce_test and test_resval_typeresolve.c link against it); add test_resval_typeresolve.c + a new controller_unit_test-style test() target — these are hand-maintained explicit file lists, not globs
src/scheduler/src/core_events.c # MODIFIED: one new trace/debug log line in xce_core_push_events()'s XCE_CORE_EVENT_RESVALS_DELTA case
src/frontend/apiserver/backend/handler.go # MODIFIED: one new trace/debug log line in OnResourceLocationUpdated
src/tools/
└── deploy-ha.sh # MODIFIED: Step 3 gains (a) a conditional apiserver-config-generation branch mirroring the existing worker/scheduler conditional, generating start-apiserver.sh (minimal boot-only config + start command — no functional auth/RBAC required, see research.md Design Decisions), and (b) an always-generated start-all.sh that sequences start.sh + health-check + whichever of start-worker.sh/start-scheduler.sh/start-apiserver.sh exist for this deploy — automates the manual recipe SKILL.md's Steps 4/4b/4c already document
.claude/skills/
├── xce-deploy-ha/
│ └── SKILL.md # MODIFIED: new Step 4d documenting the opt-in apiserver step, alongside existing Step 4b (worker/scheduler)
└── xce-test-resources/
├── SKILL.md # NEW: skill description, usage, VERDICT format (xce-test-ha convention)
└── run-resource-verify.sh # NEW: orchestrates deploy + inject + 3-layer verify (log-tail/grep for scheduler+apiserver) + verdict + cleanup
```
### Modified Files
- `src/controller/testsuite/test.c` — `set_resval()` becomes a thin wrapper around the new `resval_typeresolve.c` module; `cmd_resupdate()` gains an optional `-t <numeric|string>` flag per `-n`/`-v` pair, and, after sending the update, a follow-up per-name verification query (see Components below — Requirement 1.2/1.3's mechanism).
- `src/controller/meson.build` — `xce_test_src` (line 57) gains `testsuite/resval_typeresolve.c` so both `xce_test` and the new unit test binary can link it; a new Unity test source list/target (following the `controller_unit_test`/`sched_push_unit_test` pattern already in this file, e.g. lines 86, 215, 222) gains `testsuite/test_resval_typeresolve.c`. Both lists are hand-maintained and explicit, not glob-based — this file must be edited or the new module/test never builds.
- `src/scheduler/src/core_events.c` — `xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` case gains one trace/debug-level log line (via this component's existing `tinylog`-based logging, e.g. `TRACE`) emitting the resolved worker host, resource name, and applied value, immediately after the value is written into the worker's live state. Debug/trace level only — never fires at WARN/ERROR, never changes control flow. This is the exact site bug-1568 was root-caused at via ad hoc instrumentation; this spec makes that instrumentation permanent and machine-greppable.
- `src/frontend/apiserver/backend/handler.go` — `OnResourceLocationUpdated` (line 92) gains one `logger.Debug(...)` call (zap, per this component's existing convention) logging the resource name, host/location, and stored value, immediately after `store.ResourceLocations.Put()`.
- `src/tools/deploy-ha.sh` — Step 3 gains two additions:
1. A new conditional branch, same *opt-in-by-presence* spirit as the existing worker/scheduler branch (but checking a pre-built binary at the Go build's own output path, e.g. `/tmp/apiserver`, rather than alongside `$BINARY` — a different build system, see Components below): if present, generate `$TOP/scripts/start-apiserver.sh` (minimal boot-only config) and a `logger.yaml` with `logging.level: debug`; absent, generates nothing, so existing callers are unaffected.
2. An always-generated `$TOP/scripts/start-all.sh`: runs `start.sh` for the controller node(s), waits on Step 5's existing health-check, then runs whichever of `start-worker.sh`/`start-scheduler.sh`/`start-apiserver.sh` were actually generated for this deploy (skips any that weren't) — the same backgrounding pattern (`&`, or `setsid nohup … & disown` per SKILL.md's own noted improvement) Steps 4/4b/4c already document doing by hand.
- `.claude/skills/xce-deploy-ha/SKILL.md` — documents the new opt-in apiserver step as "Step 4d" (following the existing Step 4b worker/scheduler pattern) and `start-all.sh` as a documented shortcut for Steps 4/4b/4c combined.
### New Files
- `src/controller/testsuite/resval_typeresolve.c`/`.h` — `set_resval()`'s existing `resinfo[]` table lookup plus the new numeric-by-default fallback, moved out of `test.c` into a small, dependency-light module (no networking, no REPL state) so it is independently unit-testable. `test.c` (4298 lines, its own `main()`, `libedit`/network/REPL dependencies) cannot itself be white-box `#include`d into a Unity test the way smaller single-purpose files in this codebase are — extraction is required, not optional, to satisfy Requirement 1.1/1.4's testability.
- `src/controller/testsuite/test_resval_typeresolve.c` — Unity test program for `resval_typeresolve.c`, registered as its own `meson.build` `test()` target (see Modified Files above); this is the file task 1.1's RED tests and task 2.1's GREEN tests live in.
## System Flows
```mermaid
sequenceDiagram
participant Skill as xce-test-resources
participant Worker as xce_test (worker ctx)
participant Client as xce_test (client ctx)
participant Ctrl as controller
participant Sched as scheduler daemon
participant Api as apiserver
Skill->>Worker: join synthetic-worker
Skill->>Worker: resupdate -n <res> -v <value>
Worker->>Ctrl: xce_update_resvals (wire)
Ctrl->>Ctrl: apply to w.host.resvals
Ctrl->>Sched: notify schedUpdateResvals
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 <res> -m synthetic-worker
Client->>Ctrl: query
Ctrl-->>Skill: 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)
Api-->>Skill: apiserver-observed value (or timeout)
Skill->>Skill: compare all three vs injected value, emit verdict
```
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
| Requirement | Summary | Components | Interfaces | Flows |
|-------------|---------|------------|------------|-------|
| 1.1–1.4 | `resupdate` accepts any registered resource name | `set_resval()`/`cmd_resupdate` (test.c) | CLI flags `-n`/`-v`/`-t` | — |
| 2.1–2.2 | Inject without a real worker daemon | `xce_test` worker context (`join`, `resupdate`, existing) | CLI | Sequence diagram: join → resupdate → wire update |
| 3.1–3.2 | Controller-side visibility check | `xce-test-resources` skill, `xce_test` client context (`resval`, existing) | CLI | Sequence diagram: `resval` query → controller-observed value |
| 4.1–4.3 | Scheduler-side visibility check | `xce-test-resources` skill, scheduler trace log (`core_events.c`, new) | Log line, bounded-wait grep | Sequence diagram: scheduler applies + trace-logs → skill tails/greps scheduler log |
| 5.1, 5.2, 5.4 | Apiserver-side visibility check | `xce-test-resources` skill, `xce-deploy-ha` apiserver step, apiserver trace log (`handler.go`, new) | Log line, bounded-wait grep | Sequence diagram: apiserver stores + debug-logs → skill tails/greps apiserver log |
| 5.3 | Apiserver local deployment capability | `xce-deploy-ha` apiserver step + `start-all.sh` | Bash script | — |
| 6.1–6.5 | Repeatable run, PASS/FAIL verdict, cleanup | `xce-test-resources` skill (`run-resource-verify.sh`) | CLI, VERDICT line | Sequence diagram: final compare + verdict step |
## Components and Interfaces
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|-----------|--------------|--------|---------------|---------------------------|-----------|
| `set_resval()` / `cmd_resupdate` | Test Tooling / C | Accept any registered resource name for injection | 1.1–1.4 | `xcecli_update_resvals` (P0), `xcecli_resval` (P0) | Batch |
| Scheduler resval trace log | Scheduler Core / C | Log the resolved host/resource/value at the point the live daemon applies an update | 4.1–4.3 | `xce_core_push_events()` (P0), `tinylog` (P0) | Event |
| Apiserver resval trace log | API Server / Go | Log the resource/host/value at the point the apiserver stores a received update | 5.1, 5.2, 5.4 | `OnResourceLocationUpdated` (P0), zap `logger` (P0) | Event |
| `xce-deploy-ha` apiserver step | Deployment Tooling / Bash | Extend `deploy-ha.sh` to conditionally stand up a minimal, boot-only single-node apiserver for local verification | 5.3 | `make build-server` (P0), a running controller (P0), `deploy-ha.sh`'s existing Step 3 (P0) | Batch |
| `xce-test-resources` skill | Test Tooling / Bash | Orchestrate deploy, inject, three-layer verify (log-tail/grep for scheduler+apiserver), report, clean up | 3.1–3.2, 4.1–4.3, 5.1, 5.2, 5.4, 6.1–6.5 | `xce-deploy-ha` (P0, extended), `xce_test` (P0), scheduler/apiserver trace logs (P0) | Batch |
### Test Tooling / C
#### `set_resval()` / `cmd_resupdate`
| Field | Detail |
|-------|--------|
| Intent | Resolve a `-n <name> -v <value>` pair into a wire `xce_resval_t`, for any resource the cluster has registered |
| Requirements | 1.1, 1.2, 1.3, 1.4 |
**Responsibilities & Constraints**
- Resolve a resource name to a `Value_u_which` tag (`resval_typeresolve.c`): known non-numeric names (`type`, `model`) resolve to `Value_u_svalue`; every other name — including ones not seen before — defaults to `Value_u_fvalue`, overridable via a new optional `-t <numeric|string>` flag.
- Preserve existing behavior byte-for-byte for every currently-supported resource name (Requirement 1.4) — this is a strictly additive change to the fallback path, not a rewrite of the existing table lookup.
- Report per-name accept/reject outcome when a single invocation carries multiple `-n`/`-v` pairs (Requirement 1.3). **Confirmed via direct source inspection**: the controller's synchronous RPC reply cannot carry this signal — `update_resval_list()` (`src/controller/worker.c:185-198`) unconditionally `return 0` regardless of any per-item failure, so `do_update_resvals()`'s `reply.status` is always success even when every submitted name is unregistered. `cmd_resupdate()` therefore verifies acceptance itself: after the `xcecli_update_resvals()` call, it issues one follow-up `xcecli_resval()` query per submitted `(resource, worker)` pair — the same client-library call `cmd_resval`/`resval` already uses — and compares the returned value against what was just sent. A match is "accepted"; a not-found result or a mismatched value is "rejected", satisfying Requirement 1.2's clear per-name error without any controller-side change.
**Dependencies**
- Outbound: `xcecli_update_resvals` (`src/lib/client.c`) — unchanged, existing wire call (P0). `xcecli_resval` (`src/lib/client.c`) — unchanged, existing wire call, reused here as the accept/reject verification signal (P0).
**Contracts**: Service [ ] / API [ ] / Event [ ] / Batch [x] / State [ ]
##### Batch / Job Contract
- Trigger: operator (or the `xce-test-resources` skill) runs `resupdate -n <name> -v <value> [-t <numeric|string>] [-n <name2> -v <value2> ...]` in the `xce_test` worker context.
- Input / validation: resource name (string, required per pair), value (string, parsed per resolved type), optional type override.
- Output / destination: one `xce_update_resvals` RPC to the controller carrying every resolved pair; then one `xcecli_resval` query per pair to confirm it landed; per-pair accept/reject reported to stdout based on that follow-up query, not the update RPC's own reply (which cannot distinguish per-item outcomes — see Responsibilities above).
- Idempotency & recovery: re-running with the same name/value is safe (matches existing `resupdate` semantics — a plain overwrite, no accumulation).
**Implementation Notes**
- Integration: the type-resolution table lookup moves into `resval_typeresolve.c` unchanged in behavior; `cmd_resupdate()` gains the new post-update verification loop described above.
- Validation: the follow-up `xcecli_resval()` query adds one RPC round-trip per submitted resource name — acceptable for a test/verification tool, not a production hot path.
- Risks: a resource that is genuinely registered but whose value legitimately hasn't propagated yet (a race, not a rejection) could be misreported as "rejected" if queried immediately; mitigate with a short bounded retry on the follow-up query before concluding rejection (this codebase's existing resval-propagation timing, confirmed live during the bug-1569 investigation this spec originated from, is sub-second for a controller-local query).
### Scheduler Core / C
#### Scheduler resval trace log
| Field | Detail |
|-------|--------|
| Intent | Make the live scheduler daemon's application of a resource update observable via its own log, without a query RPC or replay |
| Requirements | 4.1, 4.2, 4.3 |
**Responsibilities & Constraints**
- Emit one trace/debug-level log line inside `xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` case (`src/scheduler/src/core_events.c`), immediately after the resolved worker's live resource value is written — including the resolved host name, resource name, and applied value, in a fixed, greppable format.
- Debug/trace level only (`TRACE`, which expands to `tlog_debug3` — `src/include/utils.h:93`) — never WARN/ERROR, never on any decision path, so normal operation is unaffected; this is purely additive observability.
- Fires on every real resource update in production too (not just this feature's synthetic injections) — acceptable, since it is the same log-volume category as this component's other existing `TRACE` calls (e.g. `apiserver_worker_change`'s `TRACE` in `src/controller/apiserver.c:446`).
- **Load-bearing operational requirement**: `tlog_debug3`/`TRACE` output is gated by the scheduler's own runtime log level (`src/scheduler/src/main.c`'s `-v` flag, cumulative — confirmed by reading `main.c`: `arguments->verbose++` per `-v`, then `verbose_to_loglevel(arguments.verbose)`). At the daemon's default (unset `-v`) log level, this line is silently never written, and the verification skill's log-grep would time out on every run — a false FAIL, not a bug in the feature under test. The skill's scheduler-deploy step MUST start the scheduler daemon with `-vvv` (verbosity 3, matching `tlog_debug3`) — the same convention `xce-deploy-ha`'s generated `start.sh` already uses for the controller (`start.sh ctrl-1 -vvv`) — see task 5.1.
**Dependencies**
- Inbound: fires as part of the existing `XCE_CORE_EVENT_RESVALS_DELTA` handling path (P0) — no new caller, no new event type.
- Outbound: `tinylog` (P0) — existing logging infrastructure, unchanged.
**Contracts**: Service [ ] / API [ ] / Event [x] / Batch [ ] / State [ ]
##### Event Contract
- Trigger: the live scheduler daemon processes a `RESVALS_DELTA` event carrying a `resupdate`-injected (or real) value.
- Output / destination: one line to the scheduler daemon's own log file.
- Idempotency & recovery: log-only; no state changes, safe to fire on every event including retries/duplicates.
### API Server / Go
#### Apiserver resval trace log
| Field | Detail |
|-------|--------|
| Intent | Make the apiserver's storage of a received resource update observable via its own log, without a REST/CLI query |
| Requirements | 5.1, 5.2, 5.4 |
**Responsibilities & Constraints**
- Add one `logger.Debug(...)` call (zap, per `error-handling.md`'s existing convention) inside `OnResourceLocationUpdated` (`src/frontend/apiserver/backend/handler.go:92-95`), immediately after `store.ResourceLocations.Put(key, *res)`, logging the resource name, location/host, and stored value.
- Debug level only — never `Error`/`Warn`, no behavior change to the store write itself.
- **Load-bearing operational requirement**: the apiserver's log level is config-driven (`config/logger.yaml`'s `logging.level`, loaded by `loadLoggerConfig()` in `src/frontend/apiserver/utils/logger.go`). Zap's default level is not Debug, so unless this config explicitly sets `level: debug`, `logger.Debug(...)` output is silently suppressed and the skill's log-grep would time out on every run — a false FAIL, not a bug in the feature under test. `xce-deploy-ha`'s apiserver step (task 3.1) MUST generate a `logger.yaml` with `logging.level: debug`.
**Dependencies**
- Inbound: fires as part of the existing `OnResourceLocationUpdated` notification-handling path (P0) — no new caller.
- Outbound: zap `logger` (P0) — existing logging infrastructure, unchanged.
**Contracts**: Service [ ] / API [ ] / Event [x] / Batch [ ] / State [ ]
##### Event Contract
- Trigger: the live apiserver process receives and stores a resource-location update pushed from the controller.
- Output / destination: one line to the apiserver's own log file.
- Idempotency & recovery: log-only; no state changes.
### Deployment Tooling / Bash
#### `xce-deploy-ha` apiserver step + `start-all.sh`
| Field | Detail |
|-------|--------|
| Intent | Extend `deploy-ha.sh` to (a) conditionally stand up a minimal, boot-only single-node apiserver against an already-deployed controller, and (b) generate a single `start-all.sh` that sequences controller → (worker/scheduler/apiserver, whichever were generated) so callers need no manual multi-script sequencing |
| Requirements | 5.3, 6.1 |
**Responsibilities & Constraints**
- Apiserver step: generate the minimal config needed for the process to **boot and connect to the controller**, plus a `logger.yaml` with `logging.level: debug` (required for task 4.2's trace line to actually be emitted — see that component's Load-bearing operational requirement), modeled on `make test-integration`'s existing `XCE_BIN`/`SRV_URL` convention. Because verification is trace-log-based (Requirement 5, see research.md Design Decisions), this config no longer needs to produce *functional* auth/RBAC — no REST query is ever issued against this instance — only enough to start the process and complete the apiserver↔controller connection that `apiserver_resource_hook_fun`/`OnResourceLocationUpdated` depends on.
- Generated **only if** a pre-built apiserver binary is already present at its conventional output path (e.g. `/tmp/apiserver`, `make build-server`'s default). **Not an exact mirror of the worker/scheduler conditional**: those binaries live in the same meson build directory as the controller and are simple presence-checked; the apiserver is a separate Go build (`make build-server`, `src/frontend/Makefile`) with its own output path, so `deploy-ha.sh` does **not** invoke `make build-server` itself — building the apiserver remains the caller's prerequisite, exactly like the existing "run `/xce-build controller` first if stale" convention in SKILL.md's Prerequisites section, just naming a different build command. This preserves the same "caller opts in by having already built the optional component" semantics the worker/scheduler conditional relies on for backward compatibility — a caller who never runs `make build-server` sees no apiserver step generated, same as one who never builds worker/scheduler today.
- `start-all.sh`: always generated (independent of which optional components exist for a given deploy). Runs `start.sh` for the controller node(s), waits on Step 5's existing health-check, then runs whichever of `start-worker.sh`/`start-scheduler.sh`/`start-apiserver.sh` were actually generated — skipping any that weren't — with the scheduler started at `-vvv` (verbosity 3) specifically so task 4.1's `TRACE`/`tlog_debug3` line is actually emitted (see that component's Load-bearing operational requirement).
- Does not attempt HA/multi-instance apiserver deployment — single instance only, sufficient for this feature's verification needs.
**Dependencies**
- Inbound: `xce-test-resources` skill (P0) — the primary consumer motivating this extension; also available to any other `xce-deploy-ha` caller by construction (same reusability posture as the existing worker/scheduler steps).
- Outbound: `make build-server` (P0), a running controller (P0), `deploy-ha.sh`'s existing Step 3/Step 5 (P0).
**Contracts**: Service [ ] / API [ ] / Event [ ] / Batch [x] / State [ ]
##### Batch / Job Contract
- Trigger: `deploy-ha.sh` invoked normally (apiserver/`start-all.sh` generation happens automatically as part of Step 3, not a separate opt-in flag the caller must pass); `start-all.sh` then invoked directly by the `xce-test-resources` skill, or by an operator.
- Input / validation: target controller host:port, a deploy/scratch directory (same as existing `xce-deploy-ha` inputs — no new required parameters).
- Output / destination: a running apiserver process connected to the controller, logs under the scratch directory (same convention as `xce-deploy-ha`'s `logs/`) — this is the log task 6.4 greps for Requirement 5's trace line.
- Idempotency & recovery: each invocation deploys fresh into a caller-specified scratch directory, matching `xce-deploy-ha`'s existing "every invocation is a fresh, self-contained deploy" convention — no in-place upgrade semantics.
**Implementation Notes**
- Integration: exact minimal-boot config shape is confirmed during implementation, not fully pinned here — see research.md Risks; scope is narrower than originally planned since functional auth/RBAC is no longer required.
- Validation: `start-all.sh`'s own controller health-check gate (reusing Step 5) must complete before it proceeds to start worker/scheduler/apiserver — the skill must additionally confirm the apiserver process has registered with the controller (connection-level, not HTTP-listener) before injecting and grepping for Requirement 5's trace line.
- Risks: apiserver startup/config requirements were previously this design's least-verified part; narrowed by dropping the REST/auth requirement, but boot-time config shape is still unconfirmed until implementation.
### Test Tooling / Bash
#### `xce-test-resources` skill
| Field | Detail |
|-------|--------|
| Intent | Deploy controller+scheduler+apiserver, inject one or more resource values via `resupdate`, verify all three layers, report PASS/FAIL, clean up |
| Requirements | 3.1, 3.2, 4.1, 4.2, 4.3, 5.1, 5.2, 5.4, 6.1, 6.2, 6.3, 6.4, 6.5 |
**Responsibilities & Constraints**
- Deploy controller, scheduler, and apiserver into a fresh scratch directory by invoking `xce-deploy-ha`'s `deploy-ha.sh` (Step 3) then its generated `start-all.sh` — no manual per-component sequencing lives in this skill; `start-all.sh` already handles controller→health-check→scheduler/apiserver ordering (see the `xce-deploy-ha` apiserver step + `start-all.sh` component above).
- Join a synthetic worker and inject one or more resource values via `resupdate` (default set: the 7 names Requirement 1 adds, plus a couple of already-supported names for regression coverage).
- Query the controller (`resval`), and tail/grep the scheduler daemon's and apiserver's own log files (bounded wait each) for the trace lines their respective resval-update handlers now emit; compare all three against the injected value and attribute any mismatch or timeout to a specific layer (Requirement 4.3's distinct-reporting requirement generalizes to all three layers).
- Emit a single machine-parseable VERDICT line, following `xce-test-ha`'s existing convention (e.g. `[xce-test-resources] VERDICT resources=7 layers_checked=3 mismatches=0 result=PASS`), plus the usual terminal PASS/FAIL oracle line.
- Clean up all deployed **processes** and runtime/data state (PIDs, sockets, controller/scheduler data directories) unconditionally — on PASS, on FAIL, and on a mid-deployment abort alike — unless the operator passes a "keep running" flag (Requirement 6.5; see Error Handling's Deployment failure entry for why this must cover the abort path too, not just a completed run). **Log files under the scratch directory are always preserved**, independent of the "keep running" flag — this is a deliberate divergence from `xce-deploy-ha`'s own `cleanup.sh` (which wipes `logs/*.log`), because the Monitoring section below promises logs survive "for post-run inspection," and this feature's own trace-log verdict evidence (the exact scheduler/apiserver lines a FAIL was based on) would otherwise be unrecoverable after a completed run.
**Dependencies**
- Inbound: none (top-level entry point, invoked directly by an operator).
- Outbound: `xce-deploy-ha` (P0, extended with the apiserver step + `start-all.sh`), `xce_test` worker/client contexts (P0), the scheduler daemon's and apiserver's own log files (P0, via the two new trace-log additions).
**Contracts**: Service [ ] / API [ ] / Event [ ] / Batch [x] / State [ ]
##### Batch / Job Contract
- Trigger: `/xce-test-resources [resource=value ...]` (defaults to Requirement 1's gap-resource set if none given).
- Input / validation: optional list of `resource=value` pairs; optional scratch directory; optional "keep running" flag.
- Output / destination: per-layer pass/fail detail (injected vs. observed value, per resource, per layer) plus one VERDICT line and the standard terminal PASS/FAIL exit code, matching `xce-test-ha`'s reporting contract closely enough to be immediately familiar to this codebase's engineers.
- Idempotency & recovery: safe to re-run — fresh scratch directory per run (`xce-deploy-ha` convention), no shared mutable state across runs.
**Implementation Notes**
- Integration: invokes `deploy-ha.sh` then its generated `start-all.sh`, which already gates scheduler/apiserver startup on the controller health-check internally — this skill does not re-implement that sequencing.
- Integration: registers cleanup via a `trap` (or equivalent) as soon as `start-all.sh` is invoked, not just at the end of a successful run — deliberately stronger than `xce-deploy-ha`'s own convention (a separate, manually-invoked `cleanup.sh`), because Requirement 6.5 requires cleanup-by-default on every path including a mid-deployment abort (see Error Handling).
- Validation: the scheduler and apiserver checks are both a bounded-wait poll of a live log file (no replay, no step/continue) — poll with a short interval up to a generous timeout, consistent with `xce-test-ha`'s existing soak-test conventions; a match found before timeout short-circuits the wait.
- Risks: timing-sensitive (bounded waits for controller→scheduler and controller→apiserver propagation); use generous, explicit timeouts, not tight polling that could flake.
## Error Handling
### Error Strategy
This feature is a verification tool, not a production service — "errors" are primarily reportable verification failures (a layer did not observe the injected value) rather than exceptions to recover from mid-run.
### Error Categories and Responses
- **Unrecognized resource name** (Requirement 1.2): `resupdate` reports a clear per-name error, sourced from its own follow-up `xcecli_resval()` verification query (see Components — the controller's update RPC reply cannot carry this signal); the skill treats this as a run-level failure with the specific rejected name(s) named in its report.
- **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.
### 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.
## Testing Strategy
- **Unit Tests**: `resval_typeresolve.c`'s type-resolution fallback — 4 cases: a currently-known numeric name (e.g. `mem`) unchanged; a currently-known string name (`type`) unchanged; an unrecognized name defaults to numeric; an unrecognized name with `-t string` override resolves to string — directly compilable and testable in isolation since the extracted module has no networking, REPL, or `test.c`-global dependency (unlike `test.c` itself, which cannot be white-box included — see File Structure Plan). The skill's verdict-aggregation/comparison step (Requirement 6) is implemented as a self-contained function over (resource, injected value, per-layer observed value) records specifically so its negative path — a genuine per-layer mismatch correctly producing an overall FAIL naming the right layer — can be verified the same way, with synthetic records, without requiring a live deployment.
- **Integration Tests**: the skill's own live run against a real controller+scheduler+apiserver deployment *is* this feature's primary integration/e2e coverage, following this codebase's established precedent for `xce-test-ha`-class tooling (soak/verification tools in this project are validated live, not via `meson test()`). At minimum, one live run per implementation change verifying: (1) a previously-missing resource name (e.g. `r15m`) is now accepted and correctly injected, (2) all three layers' trace lines report the correct value within the bounded wait, (3) a genuinely-unregistered resource name included in the same batch is reported as a specific, clearly-identified rejection (not silently dropped) and correctly drives the run's overall verdict to FAIL, per this design's Error Handling.
- **E2E Path**: the full flow in System Flows' sequence diagram, run end-to-end at least once with the default resource set before this feature is considered done.

73
.kiro/specs/resource-value-verification/requirements.md Normal file
View file

@ -0,0 +1,73 @@
# Requirements Document
## Introduction
XCE currently has no fast, self-contained way to verify that a worker's resource value change propagates correctly through the full pipeline: controller → scheduler → API server. During a recent investigation (bug-1569), engineers had to deploy full worker daemons and wait on periodic reporting cycles to reproduce and verify resource-value routing bugs, when `xce_test`'s existing `worker` context already offers a `resupdate` command capable of injecting synthetic resource values directly — but its resource-name whitelist is incomplete, and no tooling exists to confirm those values are correctly visible downstream at the scheduler or the Go API server. This feature closes the `resupdate` gap and adds an end-to-end resource-value verification capability so this class of bug can be reproduced and confirmed fixed in seconds instead of through multi-minute live-deployment investigations.
## Boundary Context (Optional)
- **In scope**: extending `xce_test`'s worker-context `resupdate` command to support any resource the cluster has registered (not just a fixed subset); a repeatable verification flow/tool that injects a resource value and confirms it is visible at the controller, scheduler, and API server layers; any deployment scripting needed to stand up a Go API server instance for this verification (none currently exists for local testing).
- **Out of scope**: changing production resource-collection behavior (`resource_sync.c`'s real load-index collection); building CI/automated-pipeline integration; fixing unrelated bugs surfaced incidentally during verification (they should be reported, not silently fixed as part of this feature); broader xce_test tooling improvements (per-command help, reference documentation, structured output, batch mode, argument-convention unification, renaming) — deliberately deferred to a separate future project, see Future Considerations below.
- **Adjacent expectations**: this feature depends on a working controller/scheduler/API-server deployment mechanism (the existing `xce-deploy-ha` skill covers controller deployment only; API-server deployment is a new capability this feature must provide or explicitly hand off). How scheduler-side (and apiserver-side) visibility is confirmed is left to the design phase, not fixed here — design.md/research.md settle on live trace-log observation at each layer's point of application, a third option not enumerated at requirements time (neither "extend the REPL flow" nor "add a new live query RPC").
## Requirements
### Requirement 1: Complete resource-name coverage in `resupdate`
**Objective:** As a developer testing XCE's resource-value pipeline, I want `xce_test`'s `resupdate` command to accept any resource name the cluster has registered, so that I can inject a synthetic value for any resource without being blocked by an incomplete hardcoded list.
#### Acceptance Criteria
1. When an operator runs `resupdate -n <resource-name> -v <value>` for a resource name currently missing from the command's supported list (including `r15m`, `pg`, `ls`, `it`, `swp`, `io`, `tmp`), the worker test context shall accept the update and report success.
2. When an operator runs `resupdate -n <resource-name> -v <value>` for a resource name that does not exist in the cluster's registered resource set, the worker test context shall report a clear error identifying the unrecognized name.
3. The worker test context shall report which resource names were accepted and which were rejected when a single `resupdate` invocation includes multiple `-n`/`-v` pairs.
4. Where a resource name is recognized, the worker test context shall preserve the resource's existing value-type behavior (numeric vs. string) exactly as before this change, for every currently-supported resource.
### Requirement 2: Inject a resource value without a real worker daemon
**Objective:** As a developer reproducing a resource-value bug, I want to set a worker's resource value through `xce_test`'s worker context alone, so that I don't need to deploy and wait on a real worker daemon's periodic reporting cycle.
#### Acceptance Criteria
1. When an operator joins a synthetic worker through `xce_test`'s worker context and then runs `resupdate`, the system shall apply the update as if it came from a real, currently-connected worker daemon, with no other worker process required.
2. The system shall tag every `resupdate`-injected value with the joined worker's own identity, consistently with how a real worker daemon's periodic report is identified.
### Requirement 3: Confirm the value is visible at the controller
**Objective:** As a developer verifying a resource-value fix, I want confirmation that an injected value is visible through the controller's own resource query, so that I know the controller-side portion of the pipeline is correct.
#### Acceptance Criteria
1. When a resource value has been injected via `resupdate`, the verification capability shall query the controller for that resource/worker pair and shall report whether the returned value matches the injected value.
2. If the controller's returned value does not match the injected value, the verification capability shall report this as a failure and shall include both the injected and observed values in its report.
### Requirement 4: Confirm the value is visible at the scheduler
**Objective:** As a developer verifying a resource-value fix, I want confirmation that an injected value is visible in the scheduler's own live resource state, so that I know the scheduler-side portion of the pipeline is correct.
#### Acceptance Criteria
1. When a resource value has been injected via `resupdate`, the verification capability shall determine, within a bounded wait time, whether that value is visible in the scheduler's own resource state for the corresponding worker.
2. If the scheduler's resource state does not reflect the injected value within the bounded wait time, the verification capability shall report this as a failure and shall include both the injected and observed values in its report.
3. The verification capability shall report the scheduler-side observed value distinctly from the controller-side observed value, so a mismatch can be attributed to a specific layer.
### Requirement 5: Confirm the value is visible at the API server
**Objective:** As a developer verifying a resource-value fix, I want confirmation that an injected value is correctly received and stored by the Go API server, so that I know the apiserver-side portion of the pipeline is correct.
#### Acceptance Criteria
1. When a resource value has been injected via `resupdate` and a Go API server instance is running against the same cluster, the verification capability shall determine, within a bounded wait time, whether that value is visible in the API server's own resource state for the corresponding worker.
2. If the API server's observed value does not match the injected value within the bounded wait time, the verification capability shall report this as a failure and shall include both the injected and observed values in its report.
3. Where no Go API server deployment mechanism currently exists for local testing, the verification capability shall provide one as part of standing up the test environment.
4. The verification capability shall report the apiserver-side observed value distinctly from the controller-side and scheduler-side observed values, so a mismatch can be attributed to a specific layer.
**Design note**: this requirement is intentionally worded like Requirement 4 (implementation-agnostic — "visible in the apiserver's own resource state," not "returned by a query") because the chosen mechanism observes the apiserver's storage layer directly (a trace log at the point of ingestion) rather than exercising its REST/CLI query path — see design.md/research.md for the rationale and the accepted trade-off (a query/serialization-layer-only regression would not be caught by this design).
### Requirement 6: Repeatable, reportable verification run
**Objective:** As a developer diagnosing or regression-testing resource-value propagation, I want a single repeatable command that runs the full injection-and-verification flow and reports a clear pass/fail result, so that I can confirm a fix (or catch a regression) without manual multi-step investigation.
#### Acceptance Criteria
1. When an operator invokes the verification capability, the system shall stand up (or reuse) a controller + scheduler + API server test deployment, inject one or more resource values, and check all three layers (controller, scheduler, API server) without further manual steps.
2. When every checked layer's observed value matches the injected value, the verification capability shall report an overall PASS.
3. When any checked layer's observed value does not match the injected value (or a layer is unreachable), the verification capability shall report an overall FAIL and shall identify which layer(s) failed.
4. The verification capability shall support checking multiple resource names (including the ones named in Requirement 1) in a single run.
5. The system shall clean up any deployment or process state it created for the verification run, unless the operator explicitly requests it be left running for further inspection.
## Future Considerations (explicitly out of scope for this project)
Raised while gathering requirements. These are real, worth doing, but the user has explicitly deferred all of them to a separate future "enhance xce_test" project — none are implemented as part of this spec:
- **Renaming `xce_test`**: as its role grows from an ad-hoc test client into a serious tool for verifying XCE controller/scheduler behavior and core architecture correctness, a more descriptive name could better communicate that purpose. Carries real blast radius (invoked by name in scripts, the `xce-test-ha`/`xce-deploy-ha` skills, and developer muscle memory) — needs its own explicit decision (candidate name, rename-vs-alias) when picked up.
- **Discoverable per-command help**: today's `help` only lists command names with no per-command usage/arguments/examples. A future project should add detailed help for every command, in every context (client/worker/scheduler).
- **Reference documentation**: a maintained written reference describing every xce_test context and command, its purpose, arguments, and an example invocation.
- **Structured (JSON) output mode** for query commands (`resval`, `joblist`, `workerlist`, etc.) so scripts/skills can parse results reliably instead of scraping human-readable tables — this session's own investigation relied entirely on fragile table-text `grep`.
- **Non-interactive batch/script mode** with clear per-command success/failure exit codes, for more robust use from skills and CI (this session drove xce_test via heredoc-piped stdin, which works but gives no clean per-command pass/fail signal).
- **Consistent argument conventions** across contexts (e.g. `resupdate`'s `-n`/`-v` flags vs. `jobsub`'s positional arguments) — a larger, more invasive cleanup with real backward-compat risk for existing scripts/skills.

102
.kiro/specs/resource-value-verification/research.md Normal file
View file

@ -0,0 +1,102 @@
# Research & Design Decisions
## Summary
- **Feature**: `resource-value-verification`
- **Discovery Scope**: Extension (existing systems: `xce_test`, controller, scheduler, apiserver)
- **Key Findings**:
- `xce_test`'s worker-context `resupdate` command already exists, already sends updates through the identical wire path a real worker daemon's periodic report uses (`xcecli_update_resvals` → `xce_update_resvals` → controller's `do_update_resvals`), and already tags `.host` correctly via `get_setting("myname")`. It is the right injection primitive for this feature — no new wire path is needed.
- `resupdate`'s resource-name coverage is gated by a hardcoded table (`set_resval()`, `src/controller/testsuite/test.c:3267`) that is missing 7 real dynamic resources (`r15m`, `pg`, `ls`, `it`, `swp`, `io`, `tmp`) present in `resource_sync.c`'s load-index set.
- The scheduler has no live (non-replay) query mechanism for its own resource state today. The only existing *query* path is `xceadmin scheduler record start/stop` + `xce_scheduler -i -s <file> --interactive` (the REPL). The REPL starts **paused** at the initial snapshot and requires an explicit `step`/`continue` before any subsequently-recorded event (including a resource update) is reflected in `info worker` output — discovered directly during this session's bug-1569 investigation, where querying immediately after load repeatedly and misleadingly looked like a bug. **This path was the original design choice and is now superseded — see Design Decisions below**: the scheduler applies the update to its own live state regardless of whether anything ever queries it, and a trace log at that point of application is both simpler and a closer match to how this feature's own originating bugs were actually found.
- No apiserver deployment mechanism exists in this repo for local/skill-driven testing today. `xce-deploy-ha` only deploys the C controller. The Go apiserver is built via `make build-server` (`src/frontend/Makefile`) → `/tmp/apiserver`, and `make test-integration` already establishes a `XCE_BIN`/`SRV_URL` convention for pointing Go-side tooling at a running controller — the closest existing precedent for this feature's apiserver deploy step.
- The apiserver already exposes a query this feature could use: `xcecli resources distribution list` → `GET /xce/v1/resources/distribution`, backed by Go's `store.ResourceLocations`. **Superseded — see Design Decisions**: the final design observes `store.ResourceLocations`'s write directly via a trace log in `OnResourceLocationUpdated`, not through this REST/CLI path, so the apiserver deploy step no longer needs functional auth/RBAC.
- On the controller, `apiserver_resource_hook_fun` (`src/controller/apiserver.c`) forwards the same raw wire resval payload to the apiserver that `sched_resource_hook_fun` forwards to the scheduler — both are fed by the same `HOOK_DETAIL_RESOURCE_UPDATE` hook `do_update_resvals()` fires. This confirms a `resupdate` injection is visible to all three layers through one single controller-side write, not three independent paths.
- **Final approach (see Design Decisions)**: both the scheduler and apiserver checks observe each layer's own point of application via a new one-line trace/debug log (`core_events.c` for the scheduler, `handler.go`'s `OnResourceLocationUpdated` for the apiserver), greppable from the skill within a bounded wait — not the REPL and not a REST/CLI query. This directly matches how bug-1568 and bug-1569(A), the two real bugs that motivated this feature, were actually root-caused.
## Research Log
### How does a resource value actually reach the scheduler and apiserver today?
- **Context**: Needed to confirm `resupdate` is a faithful stand-in for a real worker's periodic report, and to identify what each verification layer should query.
- **Sources Consulted**: `src/controller/testsuite/test.c` (`cmd_resupdate`, `set_resval`), `src/lib/client.c` (`xcecli_update_resvals`), `src/controller/opupd.c` (`do_update_resvals`), `src/controller/sched.c` (`sched_resource_hook_fun`), `src/controller/apiserver.c` (`apiserver_worker_change`, `apiserver_resource_hook_fun`), `src/scheduler/src/core_events.c` (`XCE_CORE_EVENT_RESVALS_DELTA` handling) — all read directly (not summarized) during this session's bug-1569 investigation and the follow-up research for this spec.
- **Findings**: One controller-side apply (`do_update_resvals` → `update_resval_list`) plus one hook firing (`HOOK_DETAIL_RESOURCE_UPDATE`) fans out to both the scheduler (`sched_resource_hook_fun` → `notify()` → `XCE_CORE_EVENT_RESVALS_DELTA`) and the apiserver (`apiserver_resource_hook_fun` → `notify()` to each connected apiserver instance). The controller's own `w->host.resvals` (queried via `resval` in the client context) is updated synchronously in the same call, independent of either forward.
- **Implications**: All three verification layers (Requirements 3–5) are downstream of the exact same single injection call — the design does not need three separate injection paths, only three separate read/verify steps.
### Does a live scheduler resource query already exist?
- **Context**: Requirements left "how to confirm scheduler-side visibility" as an open design decision (extend the replay/REPL path vs. add a new live admin query).
- **Sources Consulted**: `src/scheduler/src/handler.c` (`sched_handle_admin_request`, `SchedAdminAction_*`), `src/scheduler/src/repl.c`/`repl_info.c` (the `--interactive` REPL), `xceadmin`'s `scheduler record start/stop`/`scheduler dump` commands.
- **Findings**: The only scheduler-admin RPC surface today (`OPCode_schedAdmin`) covers recording control (`recordStart`/`recordStop`/`recordStatus`) — nothing exposes live resource/worker state over the wire. The only way to inspect scheduler-side state is to record a `.replay` file and load it into the offline `--interactive` REPL, which requires explicit `step`/`continue` before it reflects anything beyond the initial snapshot.
- **Implications**: See Design Decision below — adopting the existing REPL path (documented pitfall and all) instead of building a new live-query RPC.
## Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| Trace log at point of application (scheduler + apiserver) | One debug/trace log line each in `core_events.c`'s resval-delta handling and `handler.go`'s `OnResourceLocationUpdated`; skill greps each live process's log | Matches how bug-1568/1569A were actually found; no record/replay round-trip; no REPL step/continue pitfall; no apiserver auth/RBAC functional requirement | Small, deliberate production-code touch (one log line each) in scheduler and apiserver; does not exercise the real query/REST/CLI path, so a query-layer-only regression wouldn't be caught | **Selected** |
| Record+replay+REPL for scheduler; deploy+REST-query for apiserver | Record a short window, replay it, `step`/`continue`, `info worker`; separately, deploy apiserver with working auth and query via `xcecli` | Exercises the real client-facing query path; zero scheduler/apiserver code changes | REPL step/continue pitfall already produced one false negative live; apiserver auth/RBAC config previously flagged as this design's least-verified part | Original choice; superseded — see Design Decisions |
| New live scheduler admin query RPC | Add a new `SchedAdminAction` (e.g. `infoWorker`) returning live resource state | Instant, no record/replay round-trip, exercises a real query path | New wire schema (capnp) + new scheduler handler + new `xceadmin` verb — real production-code surface for a test-tooling spec | Rejected; possible future follow-up, not built here |
## Design Decisions
### Decision (SUPERSEDED — see below): Adopt the existing record+replay+REPL flow for scheduler-side verification
- **Context**: Requirement 4 needs to confirm scheduler-side visibility; requirements.md deliberately left the mechanism to design.
- **Alternatives Considered**:
1. Add a new live scheduler admin query (e.g. `SchedAdminAction_infoWorker`) that returns current resource state directly, without recording.
2. Reuse the existing `xceadmin scheduler record start/stop` + `xce_scheduler -i -s <file>` REPL flow, with explicit `step`/`continue` before querying.
- **Selected Approach (at the time)**: (2) — reuse the existing REPL flow.
- **Why superseded**: re-examined during design validation against this spec's own originating bug history (bug-1568, bug-1569A) and replaced by the trace-log decision immediately below — see that entry for the full rationale. This entry is kept for the record of what was considered and rejected the first time; do not implement against it.
### Decision: Verify scheduler- and apiserver-side visibility via live trace-log observation, not REPL replay or a REST/CLI query
- **Context**: Requirement 4 (scheduler) and Requirement 5 (apiserver) need a mechanism to confirm an injected value is visible at each layer; the record+replay+REPL approach above was the original design-phase choice, revisited after the user asked (during design validation) whether scheduler-sim-mode dependence could be avoided.
- **Key finding that changed the decision**: this feature's own originating bugs were re-examined for what kind of defect they actually were. **Bug-1568** (`.wolf/buglog.json`) — "`info worker` shows 0.00 for every resource" — was root-caused as a genuine *storage*-layer bug: `sched_add_worker()` (`src/scheduler/src/core.c:270`) is a plain `key_add()` (ADD-ONLY; fails `-EEXIST` on a worker that already exists), so a second `XCE_CORE_EVENT_WORKERS_PUSH` carrying the real values silently loses to the first, zero-valued push — the scheduler's actual in-memory state was wrong, not merely mis-displayed. It was root-caused by adding **live TRACE instrumentation directly at the point of application** (`core_events.c`), not by querying through the REPL. **Bug-1569(A)** was likewise a storage-layer bug (`resource_sync.c` tagging periodic reports with the wrong `.host` key) — and the REPL path's step/continue requirement itself produced a *false negative* during that investigation (looked broken, wasn't). Neither of the two real bugs this feature exists to catch was a query/display-layer defect; both were "the stored value is wrong," and both were actually found via a trace log at the storage site, not via REPL/REST querying.
- **Alternatives Considered**:
1. (original) Record+replay+REPL for scheduler; deploy apiserver + `xcecli resources distribution list` REST query for apiserver.
2. Add a debug/trace log line at each layer's actual point of application — scheduler: `xce_core_push_events()`'s `XCE_CORE_EVENT_RESVALS_DELTA` case in `src/scheduler/src/core_events.c` (adjacent to `sched_get_worker`); apiserver: `OnResourceLocationUpdated` in `src/frontend/apiserver/backend/handler.go:92-95` (where `store.ResourceLocations.Put()` is called). The skill greps each layer's own live log file for the expected line within a bounded wait, instead of recording/replaying (scheduler) or deploying-and-querying (apiserver).
- **Selected Approach**: (2), for both layers.
- **Rationale**: Directly matches how the actual defects in this bug class were found and is simpler on every axis — no `record`/`replay` round-trip, no REPL `step`/`continue` pitfall (already proven capable of a false negative once), no apiserver auth/RBAC config to get right (previously flagged as this design's least-verified, riskiest part), no dependency on the REST/JSON stack. The apiserver still needs to be deployed and connected to the controller (Requirement 5.3 — `OnResourceLocationUpdated` only fires on a live, connected instance), but no longer needs *functional* auth/RBAC — only enough config to boot and connect, which substantially de-risks `deploy-apiserver.sh`.
- **Trade-offs (explicitly accepted by the user)**: this checks that the value was correctly received and stored at each layer, not that a real client can retrieve it through the actual query/REST/CLI path. A future bug in the query/serialization layer specifically (as opposed to the storage layer) would not be caught by this design. Requirement 5's acceptance criteria are reworded in requirements.md to stop mandating "query the API server" as the specific mechanism, so this is a requirements-level acceptance of the trade-off, not just a design-level shortcut.
- **Follow-up**: if a future bug turns out to be in the query/serialization path rather than storage, re-add a real query-based check for that layer specifically — this decision does not preclude it.
### Decision: Default unrecognized `resupdate` resource names to numeric, keep a small explicit list for non-numeric ones
- **Context**: Requirement 1 needs `resupdate` to accept any registered resource name, not just the current hardcoded 14-entry table.
- **Alternatives Considered**:
1. Query the controller/resmgr live for each resource's registered value type before sending the update.
2. Extend the hardcoded table to enumerate every resource name across every possible cluster config.
3. Default any unrecognized name to numeric (`Value_u_fvalue`), keep only the currently-known non-numeric names (`type`, `model`) in a small explicit list, and add an optional override flag for a caller that needs a non-numeric type.
- **Selected Approach**: (3).
- **Rationale**: (1) adds a network round-trip and new query dependency to a simple CLI command for a case (wrong type guess) that is rare and already recoverable via the override flag. (2) does not scale — resource sets are cluster-config-defined, not fixed, so any static enumeration will eventually be incomplete again (as demonstrated by the 7 already-missing names). (3) is the minimal change that satisfies "accept any resource name" without new runtime dependencies, is backward compatible (existing behavior for `type`/`model` and all currently-numeric names is unchanged), and gives an explicit escape hatch instead of guessing silently wrong.
- **Trade-offs**: A brand-new non-numeric resource requires the caller to pass the override flag once; this is judged acceptable given non-numeric resources are rare in this codebase (currently only 2 of ~30+ registered resources).
- **Follow-up**: None required before implementation.
### Decision: Verify per-name accept/reject via a follow-up `xcecli_resval()` query, not the update RPC's own reply status
- **Context**: Requirement 1.2 (clear error for a genuinely unregistered resource name) and 1.3 (per-name accept/reject reporting for multi-pair invocations) need a real signal distinguishing per-item success from failure. The original design draft assumed the controller's synchronous RPC reply (`do_update_resvals`'s `reply.status`) could carry this, flagged as unconfirmed in this file's Risks — that assumption was checked directly against source during a final spec review and found to be false.
- **Confirmed finding**: `update_resval_list()` (`src/controller/worker.c:185-198`) loops over the whole batch, logs each per-item failure via `log_update_resval_failure()`, but unconditionally `return 0`. `do_update_resvals()` (`src/controller/opupd.c`) sets `reply.status` from that single return value, so the RPC reply is always "success" for the whole batch regardless of how many individual names were rejected — there is no per-name signal on this path at all, and no batch-level failure signal either.
- **Alternatives Considered**:
1. Modify `update_resval_list()`/`do_update_resvals()` to return/report per-item status on the wire.
2. Have `cmd_resupdate()` verify each submitted name itself, via a follow-up `xcecli_resval()` query (the same client-library call `resval`/`cmd_resval` already uses) per `(resource, worker)` pair, comparing the returned value against what was just sent.
- **Selected Approach**: (2).
- **Rationale**: (1) requires modifying production controller code (`worker.c`, `opupd.c`, and the RPC reply wire shape) — explicitly out of this spec's boundary, and disproportionate scope for a test-tooling fix. (2) uses only existing, unmodified client-library functions, stays entirely inside `test.c` (this spec's boundary), and gives a genuinely observable per-name result (a live query confirming — or failing to confirm — the value actually landed), which is a stronger correctness signal than a wire-level status flag would have been anyway.
- **Trade-offs**: Adds one RPC round-trip per submitted resource name (acceptable for a test tool, not a production hot path). A resource that is genuinely registered but whose value hasn't propagated to the query path yet could theoretically be misreported as rejected — mitigated with a short bounded retry before concluding rejection.
- **Follow-up**: None required before implementation; this fully resolves what was previously an open risk (see below).
### Decision: Extend `xce-deploy-ha`'s `deploy-ha.sh` with an opt-in apiserver step, instead of a new standalone `deploy-apiserver.sh`
- **Context**: this feature needs an apiserver instance connected to the controller for local verification; the original design (see design.md history) planned a brand-new, standalone `src/tools/deploy-apiserver.sh` that separately re-implemented `xce-deploy-ha`'s directory/scratch/health-check conventions "modeled on" the original rather than reusing it directly. Revisited during design validation after the user asked whether the deploy skill itself should be enhanced to support apiserver.
- **Existing precedent found**: `deploy-ha.sh` (Step 3, `.claude/skills/xce-deploy-ha/SKILL.md`) already has exactly this shape of extension for worker and scheduler — it generates `start-worker.sh`/`start-scheduler.sh` **conditionally**, only if the `xcecd_virtual`/`xce_scheduler` binaries exist alongside the controller binary at deploy time. Callers that never build worker/scheduler (e.g. `xce-test-ha`) get no such scripts and are completely unaffected. This is an established, low-risk, additive pattern for exactly this kind of "optional extra daemon" need.
- **Alternatives Considered**:
1. (original) A new, standalone `src/tools/deploy-apiserver.sh`, independently modeling `xce-deploy-ha`'s conventions.
2. Extend `deploy-ha.sh` itself: generate `start-apiserver.sh` under the same "only if the binary exists" rule already used for worker/scheduler, and document it as Step 4d in `xce-deploy-ha`'s own `SKILL.md`.
- **Selected Approach**: (2).
- **Rationale**: avoids re-implementing the same directory/cert/health-check conventions in parallel (a reuse/duplication smell the original design already half-acknowledged via "modeled on"/"mirrors" language instead of actual reuse); removes the Revalidation Trigger risk of the two scripts' conventions drifting apart over time, since there is now only one script; matches an already-precedented conditional-generation pattern (worker/scheduler), so this is idiomatic to the codebase rather than novel; and requirements.md's own "Adjacent expectations" note already floated "hand off" to `xce-deploy-ha` as a legitimate option at requirements time.
- **Trade-offs**: this feature's implementation now touches a shared, actively-used skill (`xce-deploy-ha`) instead of staying fully isolated in a new file — real blast radius, mitigated by making the apiserver step strictly opt-in/conditional (existing callers that don't build/need an apiserver see zero behavior change, exactly like the worker/scheduler precedent) and additive-only (no changes to existing controller/worker/scheduler steps).
- **Follow-up**: None required before implementation.
- **Extension (raised by the user during the same design-validation pass)**: also generate a combined `start-all.sh` in Step 3, alongside the existing per-component scripts (`start.sh`, `start-worker.sh`, `start-scheduler.sh`, and the new `start-apiserver.sh`) — it starts the controller node(s) via `start.sh`, health-checks (Step 5's existing check), then starts whichever of worker/scheduler/apiserver scripts were actually generated for this deploy, in the same order and with the same backgrounding SKILL.md's Step 4b already documents doing by hand. This is not new sequencing logic — it automates the exact manual recipe `xce-deploy-ha`'s own SKILL.md Steps 4/4b/4c already instruct a human to run — and it directly serves Requirement 6.1 ("stand up... without further manual steps") by letting the `xce-test-resources` skill call one generated script instead of re-implementing controller→scheduler/apiserver sequencing itself. Always generated (regardless of which optional components are present for a given deploy), so it has one consistent name and behavior across all callers.
## Risks & Mitigations
- No apiserver deployment mechanism exists yet, and this session never exercised the Go apiserver live. Now that verification no longer depends on a functional REST query (trace-log decision above), the remaining unknown is narrower: just enough config for the binary to boot and register with the controller (no working auth/RBAC required). Mitigation: model the new `xce-deploy-ha` apiserver step on `make test-integration`'s existing `XCE_BIN`/`SRV_URL` convention, reusing `deploy-ha.sh`'s own directory/config-generation pattern directly (see the "extend xce-deploy-ha" decision above) rather than a separate script; confirm the minimal-boot config shape during implementation.
- The scheduler's `TRACE`/`tlog_debug3` output and the apiserver's `logger.Debug` output are both gated by runtime log-level configuration (`-vvv` for the scheduler, `logger.yaml`'s `logging.level: debug` for the apiserver) — if `start-all.sh` fails to set either, the corresponding verification check times out on every run with no indication the trace line was simply never written (looks identical to a genuine propagation failure). Mitigation: this is called out explicitly as a load-bearing requirement in design.md's two trace-log component sections and in task 3.1/4.1/4.2, not left implicit.
- A trace log line added to production code (`core_events.c`, `handler.go`) must be genuinely inert in normal operation — gated at debug/trace level (never WARN/ERROR, never on the hot path for every event), consistent with each component's existing logging conventions (`tinylog` in C, zap `logger.Debug` in Go per `error-handling.md`).
- The chosen approach verifies storage-layer correctness, not the real query/REST/CLI path — accepted trade-off (see Design Decisions); a future query-layer bug would need a separate check to catch.
- Live-deployment verification (this feature's core value) is inherently timing-sensitive (bounded waits per Requirements 4/6). Mitigation: use generous, explicit timeouts consistent with this codebase's existing soak-test conventions (`xce-test-ha`), not tight polling that could flake.
- `test.c` (4298 lines, its own `main()`, heavy `libedit`/network/REPL dependencies) cannot be white-box `#include`d into a Unity test the way smaller single-purpose files in this codebase are — confirmed by direct inspection during final spec review. Mitigation: extract `set_resval()`'s type-resolution logic into a new, small, dependency-light `resval_typeresolve.c`/`.h` module that both `test.c` and a new unit test can compile against independently (see design.md File Structure Plan).
## References
- `.wolf/buglog.json` bug-1569 — the investigation that surfaced every finding in this research log, including the REPL step/continue pitfall and the incomplete `resupdate` resource table.
- `.claude/skills/xce-test-ha/SKILL.md`, `.claude/skills/xce-deploy-ha/SKILL.md` — existing skill conventions this design follows (VERDICT line format, scratch-directory deploy pattern, `setsid nohup … & disown` process backgrounding).

22
.kiro/specs/resource-value-verification/spec.json Normal file
View file

@ -0,0 +1,22 @@
{
"feature_name": "resource-value-verification",
"created_at": "2026-08-17T14:28:10Z",
"updated_at": "2026-08-18T14:35:00Z",
"language": "en",
"phase": "implementation-complete",
"approvals": {
"requirements": {
"generated": true,
"approved": true
},
"design": {
"generated": true,
"approved": true
},
"tasks": {
"generated": true,
"approved": true
}
},
"ready_for_implementation": true
}