diff --git a/.kiro/specs/resource-value-verification/tasks.md b/.kiro/specs/resource-value-verification/tasks.md index 702658d0..65b2d916 100644 --- a/.kiro/specs/resource-value-verification/tasks.md +++ b/.kiro/specs/resource-value-verification/tasks.md @@ -48,7 +48,7 @@ - Observable completion: a live scheduler daemon's log file contains one line per applied resource update, in the format task 6.3 will grep for; existing scheduler tests are unaffected - _Requirements: 4.1, 4.2, 4.3_ -- [ ] 4.2 (P) Add a trace log line to the apiserver's resource-update storage handling +- [x] 4.2 (P) Add a trace log line to the apiserver's resource-update storage handling - In `OnResourceLocationUpdated` (`src/frontend/apiserver/backend/handler.go:92-95`), add one `logger.Debug(...)` call (zap, per this component's existing convention) immediately after `store.ResourceLocations.Put(key, *res)`, logging the resource name, location/host, and stored value - Confirm existing apiserver backend tests still pass unchanged — same rationale as 4.1, no new unit test required for a pure log addition; content/format confirmed live in task 8.1 - Observable completion: a live apiserver's log file contains one line per stored resource-location update, in the format task 6.4 will grep for; existing backend tests are unaffected @@ -116,6 +116,9 @@ - _Depends: 7.1_ ## Implementation Notes +- Task 4.2: the apiserver's trace log line (task 6.4 must grep for exactly this) is zap message `"resource-location stored"` with fields `resource` (string), `host` (string), `value` (float64 for numeric resources — the common case; string for `type`/`model`; bool for boolean-typed resources), e.g. `{"resource": "r15m", "host": "review-worker", "value": 1.75}`. Grep the log for `resource-location stored`, then parse `"resource": ""` / `"host": ""` / `"value": ` from the trailing field block. +- **Task 4.2 discovered bug-1574, which will affect task 8.1 and possibly task 7.1's default resource set — read before implementing either**: `xce_test` can SEGFAULT on `resupdate -n type -v ...` / `resupdate -n model -v ...` (the two `Value_u_svalue`/string-typed resources). Root cause: `set_resval()`'s switch has no `Value_u_svalue` case, so `vals[idx].name` is left pointing at whatever garbage was already on `cmd_resupdate()`'s never-zero-initialized stack array — reading genuinely uninitialized stack memory, invisible to ASan (would need MemorySanitizer), and NON-DETERMINISTIC (task 2.2's reviewer saw `-n type -v foo` NOT crash under full ASAN; task 4.2's implementer saw `-n model -v CustomModelX` crash immediately — different argv/stack layout, same underlying UB). Task 1.2's `SKILL.md` scaffold already documents `type` as part of an illustrative default resource set "for string-path regression coverage" — **before task 7.1 finalizes the actual default resource set, either exclude `type`/`model` from any default until bug-1574 is fixed, or fix bug-1574 first** (it's a small, well-understood fix: zero-initialize `vals[]` in `cmd_resupdate()` and/or add the missing `Value_u_svalue` case to `set_resval()`'s value-population switch — out of boundary for tasks 4.1-4.2, in boundary for whichever task next touches `test.c`'s `cmd_resupdate()`/`set_resval()`, most likely as a prerequisite fix before task 8.1's live e2e run). A crash mid-run would make task 8.1's or the skill's own live verification flaky/non-deterministic, undermining the whole feature's purpose. +- Consolidated list of pre-existing (not-fixed-here) bugs surfaced by this feature's own live testing so far: bug-1571 (apiserver `RegisterToBackend` gate checks `Address` before `Peers` — worked around in generated config, not source, by task 3.1), bug-1572 (scheduler double-free/crash on a resource's 2nd periodic report — task 4.1), bug-1573 (see buglog.json, task 4.1), bug-1574 (`xce_test` segfault on string-typed `resupdate` — task 4.2, described above). - Task 4.1: the scheduler's trace log line format (task 6.3 must grep for exactly this) is `resval applied host= resource= value=` at `DEBUG3` level (requires `-vvv`, per task 3.1's `start-all.sh`). Numeric resources use `%f` (fixed 6-decimal, e.g. `12345.500000` — parse-and-compare as float, not exact string match); string-typed resources (`type`/`model`) use `%s`. Any other `Value_u_which` tag (e.g. `Value_u_bvalue`, boolean resources) is silently NOT logged by this line (matches this file's own sibling-function convention) — not relevant to this feature's resource set, which is entirely fvalue/svalue. - Task 4.1 (round 2) discovered and logged, but deliberately did NOT fix (out of boundary, confirmed pre-existing since 2026-08-03/04, unrelated to this feature): **bug-1572** — the live scheduler daemon double-frees and silently crashes on a worker's SECOND periodic resource report for the same resource (`core_events.c`'s RESVALS_DELTA case stores `rv->val` into `w->resvals` without `MOVE_PTR`, while `driver.c`'s `release_translated_payload()` unconditionally frees the same list). This feature's own resupdate-based injection flow (tasks 5.1+) should avoid resupdate-ing the exact same resource name twice against the same live scheduler within one run until bug-1572 is fixed separately, or the live e2e task (8.1) could intermittently crash the scheduler mid-run. - Task 1.1: `resval_typeresolve.c`'s new function 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. diff --git a/src/frontend/apiserver/backend/handler.go b/src/frontend/apiserver/backend/handler.go index d5465f08..694463c9 100644 --- a/src/frontend/apiserver/backend/handler.go +++ b/src/frontend/apiserver/backend/handler.go @@ -92,6 +92,26 @@ func (h *DefaultHandler) OnResourceUpdated(res *models.ResourceDefinitionInfo) { func (h *DefaultHandler) OnResourceLocationUpdated(res *models.ResourceDistributionInfo) { key := res.ResourceName + ":" + res.Location store.ResourceLocations.Put(key, *res) + + // resource-value-verification task 4.2 (Requirements 5.1, 5.2, 5.4): trace + // the stored value at its point of application, mirroring the scheduler's + // sibling trace log added in task 4.1 (core_events.c). One typed "value" + // 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. + var valueField zap.Field + switch res.ValueType { + case int(models.STRING): + valueField = zap.String("value", res.StringValue) + case int(models.BOOLEAN): + valueField = zap.Bool("value", res.BooleanValue) + default: + valueField = zap.Float64("value", res.NumericValue) + } + logger.Debug("resource-location stored", + zap.String("resource", res.ResourceName), + zap.String("host", res.Location), + valueField) } // dispatchJobArray delivers a locally-expanded compact array push to the