feat(resource-value-verification): verdict aggregation, reporting, cleanup (task 7.1)

aggregate_verdict()/build_verdict_record() combine the three per-layer check results into a single per-resource comparison, self-contained and invokable with synthetic records (task 8.2). Uses ASCII Unit Separator (\x1f) as the record delimiter after discovering bash's IFS tab-splitting silently collapses adjacent empty fields even with IFS set to only a tab (bug-1581), which corrupted UNREACHABLE-record field alignment. Emits one machine-parseable VERDICT line (resources/layers_checked/ mismatches/unreachable/rejected/result) plus the terminal PASS/FAIL oracle line and matching exit code. Cleanup is the same trap-based mechanism task 5.1 registered for the abort path - logs always preserved regardless of --keep-running. Round-2 fix: main()'s live-data wiring (build_verdict_records_from_tsv()) derived a layer's status from array-key existence rather than the actual recorded outcome - since tasks 6.3/6.4 always set the key (even on a genuine timeout, just with an empty value), every real UNREACHABLE was silently misclassified as MISMATCH. Fixed to check SCHEDULER_CHECK_RESULTS/ APISERVER_CHECK_RESULTS string content instead (bug-1582). Live-confirmed by killing the scheduler mid-run: VERDICT now correctly reports unreachable=9 mismatches=0, not the reverse. SKILL.md rewritten to describe the now-complete, functional tool. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-18 13:18:01 +00:00 · 2026-08-18 13:18:01 +00:00 · 1364664769
commit 1364664769
parent 0253b4ca09
3 changed files with 579 additions and 173 deletions
--- a/.claude/skills/xce-test-resources/SKILL.md
+++ b/.claude/skills/xce-test-resources/SKILL.md
@ -6,80 +6,70 @@ model: haiku

 # XCE Resource-Value Verification

-## Status: deployment step functional; injection/verification not yet built
+## Status: complete and functional (tasks 1.1-7.1 all done)

-`run-resource-verify.sh` now exists (task 5.1) and implements the
-**deployment step only**: it invokes `xce-deploy-ha`'s `deploy-ha.sh` into a
-fresh scratch directory, then its generated `start-all.sh`, confirms
-controller + scheduler (`-vvv`) + apiserver (debug logging) are all
-genuinely up, and cleans up (trap-based, covers a mid-deployment abort, not
-just a completed run — see Error handling below). Invoking
-`/xce-test-resources` today runs a real deploy-confirm-cleanup cycle but
-does **not yet** inject a resource value or check any layer — it exits 0 on
-a successful deployment/cleanup and non-zero on a deployment failure, not on
-an injection PASS/FAIL verdict. Report this gap rather than fabricate a
-verdict if asked to check actual resource-value propagation.
-
-The rest of the orchestration logic is built incrementally by later tasks in
-the same spec:
- **Task 2.1/2.2** — `resupdate` accepts any registered resource name and
-  reports per-name accept/reject (closes the gap this whole feature exists to
-  close; `resval_typeresolve.c`'s fallback path). Done.
- **Task 3.1** — `xce-deploy-ha`'s `deploy-ha.sh` grows a conditional apiserver
-  step + a combined `start-all.sh` (controller → scheduler → apiserver). Done.
- **Task 4.1/4.2** — one trace-log line each in the scheduler
-  (`core_events.c`, `XCE_CORE_EVENT_RESVALS_DELTA`) and the apiserver
-  (`handler.go`'s `OnResourceLocationUpdated`) — what this skill will
-  tail/grep for. Done.
- **Task 5.1** — wires `start-all.sh` + trap-based cleanup into this skill.
-  Done (this is the deployment step described above).
- **Task 6.1–6.4** — worker join + injection, and the controller/scheduler/
-  apiserver checks themselves. Not yet built.
- **Task 7.1** — verdict aggregation, the VERDICT line, and cleanup-on-every-path.
-  Not yet built.
-
-Once tasks 6.1–7.1 land, this section (and the rest of this document's
-hedging about the VERDICT line/PASS-FAIL oracle being aspirational) should
-be removed.
-
-## What this skill will do (target behavior)
+`run-resource-verify.sh` implements the full flow described below: deploy
+controller + scheduler + apiserver, join a synthetic worker, inject one or
+more resource values, verify all three layers, emit a VERDICT line plus the
+standard terminal PASS/FAIL oracle line, and clean up. Invoking
+`/xce-test-resources` runs the whole thing end-to-end and its exit code
+reflects the real overall verdict (0 = PASS, non-zero = FAIL) -- not just
+whether the deployment stood up.

 Bug-1569 required deploying real worker/controller/scheduler/apiserver
 daemons and waiting on periodic reporting cycles just to confirm a
 resource-value routing fix. This skill turns that multi-minute manual
-investigation into one command:
+investigation into one command.
+
+## What this skill does

 1. **Deploy** controller + scheduler + apiserver into a fresh scratch
   directory by invoking `xce-deploy-ha`'s `deploy-ha.sh` (Step 3) and then
-   its generated `start-all.sh` — no manual per-component sequencing lives in
-   this skill; `start-all.sh` already gates scheduler/apiserver startup on
-   `xce-deploy-ha`'s existing controller health-check.
-2. **Inject** — join a synthetic worker through `xce_test`'s worker context
+   its generated `start-all.sh` -- no manual per-component sequencing lives
+   in this skill; `start-all.sh` already gates scheduler/apiserver startup
+   on `xce-deploy-ha`'s existing controller health-check. A `trap`-based
+   cleanup is registered before the first process starts, so it fires on
+   every exit path -- a completed PASS/FAIL run, a mid-deployment abort, or
+   an external SIGINT/SIGTERM -- not just a successful completion.
+2. **Inject** -- join a synthetic worker through `xce_test`'s worker context
   (no real worker daemon process) and run `resupdate -n <name> -v <value>`
-   for one or more resource names.
-3. **Verify all three layers**:
-   - **Controller** — query via `xce_test`'s client-context `resval` and
-     compare against the injected value.
-   - **Scheduler** — tail/grep the live scheduler daemon's own log file for
-     the trace line task 4.1 adds, with a short poll interval up to a
-     generous bounded wait (no record/replay/REPL).
-   - **Apiserver** — tail/grep the apiserver's own log file for the trace
-     line task 4.2 adds, same bounded-wait posture.
-   Each layer's observed value is reported distinctly, so a mismatch is
-   attributable to a specific layer, not just "something failed."
-4. **Report** a single VERDICT line plus the standard terminal PASS/FAIL
-   oracle line and exit code.
+   for one or more resource names in a single RPC round trip. Every
+   submitted name gets a definitive "accepted" or "rejected: \<reason\>"
+   outcome.
+3. **Verify all three layers**, for every accepted name:
+   - **Controller** -- query via `xce_test`'s client-context `resval` and
+     compare against the injected value (run nested inside the still-open
+     injecting worker session, so host-attrs resources like `cpuf` are
+     never reset by a premature disconnect -- see bug-1579).
+   - **Scheduler** -- tail/grep the live scheduler daemon's own log file
+     (`resval applied host=<H> resource=<R> value=<V>`, `-vvv`) with a short
+     poll interval up to a 15s bounded wait (no record/replay/REPL).
+   - **Apiserver** -- tail/grep the apiserver's own log file (zap
+     `"resource-location stored"` with a trailing JSON field block), same
+     bounded-wait posture.
+   Each layer's observed value is reported distinctly (MATCH / MISMATCH /
+   UNREACHABLE), so a failure is attributable to a specific layer, not just
+   "something failed" -- UNREACHABLE (bounded wait timed out, no trace line
+   ever appeared) is always reported distinctly from MISMATCH (a trace line
+   appeared with the wrong value).
+4. **Aggregate and report** -- combine the injection accept/reject outcome
+   and all three layers' results into one machine-parseable VERDICT line
+   plus the standard terminal PASS/FAIL oracle line (see below). A rejected
+   resource name is a run-level failure in its own right, independent of
+   whatever the (nonexistent, since a rejected name is never actually
+   applied anywhere) per-layer results would say.
 5. **Clean up** every process and runtime/data-directory artifact this run
-   created — unconditionally, on PASS, FAIL, or a mid-deployment abort alike
-   — unless the operator explicitly asked for the deployment to be left
-   running. **Log files under the scratch directory are always preserved**,
-   independent of that flag, so a FAIL verdict's underlying trace-log
-   evidence stays inspectable after the run (a deliberate divergence from
-   `xce-deploy-ha`'s own `cleanup.sh`, which wipes `logs/*.log`).
+   created -- unconditionally, on PASS, FAIL, or a mid-deployment abort
+   alike -- unless the operator explicitly asked for the deployment to be
+   left running (`--keep-running`). **Log files under the scratch directory
+   are always preserved**, independent of that flag, so a FAIL verdict's
+   underlying trace-log evidence stays inspectable after the run (a
+   deliberate divergence from `xce-deploy-ha`'s own `cleanup.sh`, which
+   wipes `logs/*.log`).

-**Idempotency & recovery**: every invocation is safe to re-run — a fresh
+**Idempotency & recovery**: every invocation is safe to re-run -- a fresh
 scratch directory per run, following `xce-deploy-ha`'s existing "every
-invocation is a fresh, self-contained deploy" convention — with no shared
+invocation is a fresh, self-contained deploy" convention -- with no shared
 mutable state carried across runs.

 ## Usage
@ -91,124 +81,176 @@ run-resource-verify.sh [--keep-running] [--dir DIR] [resource=value ...]
 (the `/xce-test-resources [resource=value ...]` form above dispatches to this
 script.)

- No arguments: defaults to Requirement 1's gap-resource set — the 7 names
-  `resupdate` is currently missing (`r15m`, `pg`, `ls`, `it`, `swp`, `io`,
-  `tmp`) — plus a couple of already-supported names for regression coverage
-  (e.g. `mem` for the numeric path, `type` for the string path), so a default
-  run also proves the fix didn't disturb existing behavior.
+- No arguments: defaults to `DEFAULT_RESOURCE_ARGS` -- Requirement 1's
+  gap-resource set (`r15m`, `pg`, `ls`, `it`, `swp`, `io`, `tmp`) plus two
+  already-supported NUMERIC names for regression coverage (`mem`, `cpuf`),
+  so a default run also proves the `resupdate` fix didn't disturb existing
+  behavior. Deliberately excludes `type`/`model` (string-typed) from the
+  default set -- see bug-1574 below.
 - One or more `resource=value` pairs: checks exactly those, e.g.
-  `/xce-test-resources r15m=1.5 pg=3`. **Not yet implemented** — task 5.1
-  parses and accepts these but does not act on them yet; injection lands in
-  task 6.1.
- `--dir DIR` — scratch/deploy directory. Default: a fresh directory
+  `/xce-test-resources r15m=1.5 pg=3`. A name the cluster doesn't recognize
+  is reported as a rejection and drives the overall verdict to FAIL,
+  identifying the specific name -- it is never silently dropped or ignored.
+- `--dir DIR` -- scratch/deploy directory. Default: a fresh directory
  generated via `mktemp -d` (xce-deploy-ha's "every invocation is a fresh,
  self-contained deploy" convention), or `$XCE_RESVERIFY_TOP` when set and
  `--dir` is not given (same precedence pattern as `xce-test-ha`'s
  `$XCE_HA_TOP`).
- `--keep-running` — skip PROCESS teardown in cleanup, leaving the
+- `--keep-running` -- skip PROCESS teardown in cleanup, leaving the
  deployment running under `DIR` for further inspection. Log files under
-  `DIR` are ALWAYS preserved regardless of this flag (see Error handling
-  below); it only controls whether controller/scheduler/apiserver processes
-  are killed.
+  `DIR` are ALWAYS preserved regardless of this flag; it only controls
+  whether controller/scheduler/apiserver processes are killed. The verdict
+  is still computed and reported normally either way -- `--keep-running`
+  does not change PASS/FAIL or the exit code, only what happens to the
+  processes afterward. Tear the deployment down manually afterward (its PIDs
+  are visible via `ps`/`pgrep` scoped to `DIR`).
 - Other overridable env vars (all optional, default to this repo's own
  meson build dir / `/tmp/apiserver`): `$XCE_RESVERIFY_BUILD_DIR`,
  `$XCE_RESVERIFY_BIN` (controller), `$XCE_RESVERIFY_SCHED_BIN`,
-  `$XCE_RESVERIFY_APISERVER_BIN`, `$XCE_RESVERIFY_PEERS` (defaults to a
-  single-node `localhost:9991` — this skill verifies value propagation, not
-  raft/HA failover, so a single node is deliberate and avoids
-  election-timing flakiness; xce-test-ha remains the tool for HA testing).
+  `$XCE_RESVERIFY_APISERVER_BIN`, `$XCE_RESVERIFY_XT` (the `xce_test`
+  binary), `$XCE_RESVERIFY_PEERS` (defaults to a single-node
+  `localhost:9991` -- this skill verifies value propagation, not raft/HA
+  failover, so a single node is deliberate and avoids election-timing
+  flakiness; xce-test-ha remains the tool for HA testing).
+
+Must be invoked under an environment with `nix` on `PATH` -- the script
+shells out to `nix develop` itself for the steps that need it, so it does
+not need to be pre-wrapped by the caller.

 ## VERDICT line and PASS/FAIL oracle

-Following `xce-test-ha`'s machine-parseable VERDICT convention:
+Following `xce-test-ha`'s machine-parseable VERDICT convention, emitted by
+`aggregate_verdict()`:

 ```
-[xce-test-resources] VERDICT resources=7 layers_checked=3 mismatches=0 result=PASS
+[xce-test-resources] VERDICT resources=9 layers_checked=3 mismatches=0 unreachable=0 rejected=0 result=PASS
 ```

- `resources` — count of resource names checked this run (7 for the
-  canonical Requirement-1-gap-only run shown above; a default no-args run
-  that also includes the regression names, or an explicit `resource=value`
-  list, reports whatever count was actually checked).
- `layers_checked` — always 3 (controller, scheduler, apiserver) for a full
-  run.
- `mismatches` — count of (resource, layer) pairs where the observed value
-  did not match the injected value, or the layer was unreachable
-  (trace-log grep timed out). `result=PASS` iff `mismatches == 0`.
- `result` — `PASS` or `FAIL`.
+- `resources` -- total count of resource names checked this run (accepted
+  and rejected alike).
+- `layers_checked` -- always 3 (controller, scheduler, apiserver).
+- `mismatches` -- count of (resource, layer) pairs where a trace
+  line/query DID return a value but it did not match the injected value
+  (within numeric tolerance; exact string match for non-numeric values).
+- `unreachable` -- count of (resource, layer) pairs where the bounded wait
+  timed out with NO trace line ever observed -- distinct from `mismatches`
+  per Requirement 4.3/5.4 (the two point at different bugs: a
+  value-propagation defect vs. a layer that never came up or never logged).
+- `rejected` -- count of resource names the injection step itself rejected
+  (Requirement 1.2) -- never checked at any layer, since nothing was ever
+  applied anywhere for a rejected name.
+- `result` -- `PASS` iff `mismatches == 0 AND unreachable == 0 AND
+  rejected == 0`; `FAIL` otherwise.

-Per-resource, per-layer detail (injected vs. observed value) is reported
-above the VERDICT line for anything that mismatched, distinguishing a genuine
-**mismatch** (wrong value observed) from **unreachable** (bounded wait timed
-out with no matching trace line) — the same distinction Requirement 4.3/5.4
-and design.md's Error Handling require, since the two point at different bugs
-(a value-propagation defect vs. a layer that never came up or never logged).
-
-Alongside the VERDICT line, the skill emits the usual terminal oracle line so
-a goal-loop or CI-style caller can branch on it without parsing the VERDICT
-line's fields:
+Immediately above the VERDICT line, one `FAILURE:` line is printed per
+failing (resource, layer) pair or rejected name, naming the specific
+resource and layer (or `layer=injection` for a rejection) plus the injected
+and observed values -- e.g.:

 ```
-[xce-test-resources] PASS: 7 resources, 3 layers checked, zero mismatches
-[xce-test-resources] FAIL: resource=r15m layer=scheduler mismatch (injected=1.5 observed=timeout)
+[xce-test-resources] FAILURE: resource=r15m layer=scheduler status=MISMATCH injected=1.5 observed=1.2
+[xce-test-resources] FAILURE: resource=not-a-real-resource layer=injection status=REJECTED detail="rejected: unknown resource"
 ```

-PASS ⇒ exit `0`. Any FAIL condition ⇒ non-zero exit.
+Then exactly one terminal oracle line:

-## Error handling this skill will implement
+```
+[xce-test-resources] PASS: 9 resource(s), 3 layers checked, zero mismatches, zero unreachable, zero rejected
+[xce-test-resources] FAIL: 2 resource(s), 3 layers checked, 0 mismatch(es), 0 unreachable, 1 rejected -- see FAILURE lines above
+```
+
+PASS ⇒ exit `0`. Any FAIL condition ⇒ exit `1`. A structural failure before
+the aggregation step is reached (deployment failure, worker-join failure, a
+malformed `--dir`/argument, etc.) still exits non-zero via this script's
+`fail()`/`usage()` (1, or 2 for a usage/argument error) -- unrelated to the
+verdict's own `result=` field, since aggregation never ran.
+
+A full run against the default resource set produces **exactly one** VERDICT
+line and a matching process exit code; no controller/scheduler/apiserver
+processes remain afterward (unless `--keep-running` was passed); the run's
+log files are present on disk regardless.
+
+### The comparison/aggregation step is self-contained and independently testable
+
+`aggregate_verdict()` (and its `build_verdict_record()` formatting helper)
+takes plain records as function arguments -- no live command, no file I/O,
+no global deployment state -- so it can be invoked directly with
+hand-written synthetic records, with no live deployment required. This is
+what task 8.2's dedicated test harness exercises; source the script
+(`source run-resource-verify.sh`) to get `aggregate_verdict()`,
+`build_verdict_record()`, and `controller_values_match()` in scope without
+triggering `main()` or any live deployment -- the bottom of the file is
+guarded (`[[ "${BASH_SOURCE[0]}" == "${0}" ]]`) specifically so sourcing it
+has no side effects.
+
+## Error handling this skill implements

 Per design.md's Error Handling section:

- **Unrecognized resource name** — `resupdate`'s own follow-up
-  `xcecli_resval()` verification (task 2.2) reports it as rejected; the skill
-  treats this as a run-level failure and names the specific rejected
-  resource(s) in its report.
- **Layer mismatch** — reported with both the injected and observed value,
+- **Unrecognized resource name** -- `resupdate`'s own follow-up
+  `xcecli_resval()` verification (task 2.2) reports it as rejected; the
+  aggregation step (task 7.1) treats this as a run-level failure and names
+  the specific rejected resource(s) in its FAILURE lines.
+- **Layer mismatch** -- reported with both the injected and observed value,
  attributed to the specific layer (controller/scheduler/apiserver).
 - **Layer unreachable** (e.g. apiserver never came up, or the expected
-  trace line never appears before the bounded wait elapses) — treated the
-  same as a mismatch for verdict purposes (FAIL), but labeled "timeout"
-  distinctly from "mismatch" in the report, never a silent skip.
- **Deployment failure** (controller/scheduler/apiserver fails to start) —
+  trace line never appears before the bounded wait elapses) -- treated the
+  same as a mismatch for verdict purposes (FAIL), but labeled UNREACHABLE
+  distinctly from MISMATCH in the report, never 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: a trap (or
-  equivalent) is installed covering every deployment step from the first
-  process start onward, not just the post-injection verification path —
-  deliberately stronger than `xce-deploy-ha`'s own precedent (a separate,
-  manually-invoked `cleanup.sh` that a mid-deploy failure would otherwise
-  leave for the caller to run by hand).
+  failed at step X" message. Cleanup still runs on this path: a trap
+  registered before the first process starts covers every deployment step
+  from the first process start onward, not just the post-injection
+  verification path.

-## Not using `context: fork` for the eventual orchestration script
+## Known pre-existing bugs relevant to this skill (not fixed here)

-Neither this skill's frontmatter nor the invocation instructions below will
-set `context: fork`, for the same reason `xce-test-ha` and `xce-deploy-ha`
-both call out explicitly in their own `SKILL.md`s: this skill's
-`run-resource-verify.sh` will invoke `xce-deploy-ha`'s `start-all.sh`
-internally, starting real backgrounded controller/scheduler/apiserver
-processes for the duration of the run. `context: fork`'s own teardown sends
-SIGHUP through the whole process group on return — the same confirmed-broken
-mechanism `xce-test-ha`'s soak driver and `xce-deploy-ha` both hit (bug
-confirmed 2026-07-08) — that would silently kill those processes mid-run,
-before cleanup or the verdict line ever fires. Dispatch it directly via Bash,
-or via the `Agent` tool
-(empirically verified safe for this exact pattern — see `xce-test-ha`'s and
+- **bug-1572** (open): the live scheduler daemon can double-free/crash if
+  the SAME resource is re-pushed to it a second time while it's running
+  (e.g. left alive for a while under `--keep-running`, or a controller
+  periodic resync). A single run's own injection (one `resupdate` per
+  resource, one verification pass) does not trigger this, but don't rely on
+  a `--keep-running` deployment staying up indefinitely once left running.
+- **bug-1574** (open): `xce_test` can non-deterministically SEGFAULT on a
+  string-typed `resupdate` (`-n type`/`-n model`) -- uninitialized-memory
+  UB, not this feature's own bug. This is why the default resource set is
+  numeric-only; avoid adding `type`/`model` to a custom `resource=value`
+  argument until this is fixed.
+- **bug-1579** (fixed): host-attrs resources (`cpuf`, `maxcpus`, etc.) used
+  to false-MISMATCH at the controller layer because the synthetic worker
+  disconnected before the controller-side check ran. Fixed by nesting the
+  controller check inside the still-open injecting worker session.
+
+## Not using `context: fork` for the orchestration script
+
+Neither this skill's frontmatter nor the invocation instructions above set
+`context: fork`, for the same reason `xce-test-ha` and `xce-deploy-ha` both
+call out explicitly in their own `SKILL.md`s: `run-resource-verify.sh`
+starts real backgrounded controller/scheduler/apiserver processes for the
+duration of the run, and `context: fork`'s own teardown sends SIGHUP through
+the whole process group on return -- the same confirmed-broken mechanism
+`xce-test-ha`'s soak driver and `xce-deploy-ha` both hit -- which would
+silently kill those processes mid-run, before cleanup or the verdict line
+ever fires. Dispatch it directly via Bash, or via the `Agent` tool
+(empirically verified safe for this exact pattern -- see `xce-test-ha`'s and
 `xce-deploy-ha`'s own `SKILL.md`s), not via a forked Skill-tool context.

 ## Dependencies

 - `xce_test`'s worker-context `resupdate`/`join` and client-context `resval`
-  — resource-name table gap closed (task 2.1/2.2, done).
- `xce-deploy-ha`'s `deploy-ha.sh` / `start-all.sh` — apiserver step and
-  combined `start-all.sh` done (task 3.1).
- Scheduler and apiserver trace-log lines — done (tasks 4.1 and 4.2).
- This skill's own `run-resource-verify.sh` — deployment step done (task
-  5.1); injection + per-layer checks + verdict aggregation not yet built
-  (tasks 6.1–7.1).
+  -- resource-name table gap closed (tasks 2.1/2.2).
+- `xce-deploy-ha`'s `deploy-ha.sh` / `start-all.sh` -- apiserver step and
+  combined `start-all.sh` (task 3.1).
+- Scheduler and apiserver trace-log lines (tasks 4.1 and 4.2).
+- This skill's own `run-resource-verify.sh` -- deployment (task 5.1),
+  injection + per-layer checks (tasks 6.1-6.4), and verdict aggregation +
+  cleanup-on-every-path (task 7.1) -- all done.

 ## Requirements traceability

-This stub documents the contract for design.md's "`xce-test-resources`
-skill" component (Components and Interfaces → Test Tooling / Bash), covering
-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.
-See `.kiro/specs/resource-value-verification/requirements.md` and `design.md`
+This document describes design.md's "`xce-test-resources` skill" component
+(Components and Interfaces → Test Tooling / Bash), covering 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. See
+`.kiro/specs/resource-value-verification/requirements.md` and `design.md`
 for the full text.
--- a/.claude/skills/xce-test-resources/run-resource-verify.sh
+++ b/.claude/skills/xce-test-resources/run-resource-verify.sh
@ -5,12 +5,14 @@
 # resource value via xce_test's resupdate, confirm it is visible at the
 # controller, live scheduler, and Go apiserver).
 #
-# resource-value-verification spec, task 5.1+6.1+6.2+6.3+6.4 SCOPE (+
-# bug-1579 follow-up fix): this file currently implements deployment (task
-# 5.1), worker join + resource injection (task 6.1), the controller-side
-# per-layer verification check (task 6.2), the scheduler-side per-layer
-# verification check (task 6.3), AND the apiserver-side per-layer
-# verification check (task 6.4) -- stand up a fresh controller + scheduler
+# resource-value-verification spec, task 5.1+6.1+6.2+6.3+6.4+7.1 SCOPE (+
+# bug-1579 follow-up fix): this file implements the FULL skill -- deployment
+# (task 5.1), worker join + resource injection (task 6.1), the
+# controller-side per-layer verification check (task 6.2), the
+# scheduler-side per-layer verification check (task 6.3), the apiserver-side
+# per-layer verification check (task 6.4), AND the final verdict aggregation
+# / VERDICT line / PASS-FAIL oracle / cleanup-on-every-path (task 7.1) --
+# stand up a fresh controller + scheduler
 # (-vvv) + apiserver (debug logging) deployment via xce-deploy-ha's
 # deploy-ha.sh + generated start-all.sh, confirm all three components are
 # actually up, join a SYNTHETIC worker (no real worker daemon process)
@ -34,12 +36,29 @@
 # internal_controller_check() function and the top-of-file bug-1579 note
 # below for why this matters and exactly how it works; DEFAULT_RESOURCE_ARGS
 # now includes `cpuf` again as proof the fix is real (see that note).
-# Verdict aggregation / cleanup-on-every-path (task 7.1) is added by a LATER
-# task -- see the "Extension point" marker in main() below for exactly where
-# it hooks in.
 # See .kiro/specs/resource-value-verification/{requirements,design}.md,
-# Requirements 2.1, 2.2, 3.1, 3.2, 4.1, 4.2, 4.3, 5.3, 6.1, 6.4, 6.5 for these
-# tasks; the full skill covers 3.1-6.5.
+# Requirements 2.1, 2.2, 3.1, 3.2, 4.1, 4.2, 4.3, 5.3, 6.1, 6.2, 6.3, 6.4,
+# 6.5 for these tasks; the full skill covers 3.1-6.5 and is now complete.
+#
+# Task 7.1 (this revision): adds the final aggregation step. Combines the
+# controller/scheduler/apiserver per-layer results (tasks 6.2-6.4) plus the
+# injection accept/reject outcome (tasks 2.1/2.2/6.1) into a single overall
+# verdict, via a SELF-CONTAINED comparison/aggregation function
+# (aggregate_verdict(), see its own header comment below) that takes plain
+# TSV "records" as arguments -- no live command, no file I/O, no global
+# deployment state -- so it is directly callable with hand-written synthetic
+# records for testing (this task's own sanity-check invocations, and task
+# 8.2's dedicated harness, which depends on this exact function/record
+# format) as well as from main()'s live-run path. Emits exactly one
+# machine-parseable VERDICT line (xce-test-ha's convention) plus the
+# standard terminal PASS/FAIL oracle line; exit code is 0 for an overall
+# PASS, 1 for an overall FAIL -- this is now the script's REAL exit code,
+# not the deployment-only placeholder described in the "Exit 0/Non-zero"
+# note below the Usage block (kept for historical/task-5.1-6.4 context, see
+# the correction directly beneath it). The bottom of this file is also now
+# guarded so it can be `source`d (e.g. by task 8.2's harness) without
+# executing main()/internal_controller_check() as a side effect -- see the
+# `[[ "${BASH_SOURCE[0]}" == "${0}" ]]` guard at the very end.
 #
 # Task 6.1 deliberate deviation from design.md: the default (no-args)
 # resource set is Requirement 1's gap-resource set (r15m, pg, ls, it, swp,
@ -126,6 +145,8 @@
 #                       coverage (see the top-of-file bug-1574 note for why
 #                       the default deliberately excludes type/model).
 #
+# --- SUPERSEDED by task 7.1 (kept for historical context -- this described
+#     the script's exit code BEFORE the final aggregation step existed) ---
 # Exit 0 == this task's scope succeeded (deployment stood up, all three
 # components confirmed up, worker joined, resupdate ran for every requested
 # resource -- accepted AND rejected outcomes are both a successful run of
@ -148,6 +169,21 @@
 # MISMATCH/UNREACHABLE yet (see ANY_CONTROLLER_MISMATCH/
 # ANY_SCHEDULER_MISMATCH/ANY_SCHEDULER_UNREACHABLE/ANY_APISERVER_MISMATCH/
 # ANY_APISERVER_UNREACHABLE, groundwork for task 7.1's aggregation).
+# --- CURRENT (task 7.1, this revision) ---
+# Exit 0 == overall verdict PASS: every requested resource name was accepted
+# (Requirement 1.2's rejection path never fired) AND every accepted
+# resource's controller/scheduler/apiserver observed value matched its
+# injected value (see aggregate_verdict() below) -- exactly what the
+# script's single VERDICT line's `result=PASS` reports.
+# Non-zero (1) == overall verdict FAIL: at least one requested resource name
+# was rejected, OR at least one accepted resource's value did not match (or
+# was UNREACHABLE/timed out) at some layer -- exactly what `result=FAIL`
+# reports, with every specific (resource, layer) failure named in the
+# FAILURE lines aggregate_verdict() logs immediately above the VERDICT line.
+# A structural failure before aggregation is reached (deployment failure,
+# worker-join failure, a resupdate with no observable accept/reject outcome,
+# a malformed `--dir`/argument, etc.) still exits non-zero via fail()/usage()
+# (1, or 2 for a usage/argument error) -- unchanged from before this task.
 #
 set -u

@ -1133,6 +1169,318 @@ query_apiserver_resvals() {
    done
 }

+# ── Step 8: verdict aggregation, VERDICT line, PASS/FAIL oracle (task 7.1) ──
+# Requirement 6.2/6.3/6.4: combine the controller/scheduler/apiserver
+# per-layer results (tasks 6.2-6.4) plus the injection accept/reject outcome
+# (tasks 2.1/2.2/6.1) into a single overall verdict. Deliberately
+# implemented as a SELF-CONTAINED comparison/aggregation function --
+# aggregate_verdict() below takes an array of plain TSV "records" (one per
+# checked resource, see the format comment on aggregate_verdict() itself)
+# and does its own MATCH/MISMATCH/UNREACHABLE classification via
+# controller_values_match() (task 6.2's own pure comparison helper, reused
+# here rather than re-parsing/re-trusting each layer's already-rendered
+# CHECK_RESULTS string) -- it runs no live command, reads no file, and
+# touches no global deployment state, so it is directly callable with
+# hand-written SYNTHETIC records for testing (this task's own sanity-check
+# invocations during implementation, and task 8.2's dedicated harness, which
+# depends on this exact function/record format). main() below builds the
+# record array from the ALREADY-POPULATED INJECTED_VALUES/INJECTED_RESULTS
+# (task 6.1) and CONTROLLER_OBSERVED/SCHEDULER_OBSERVED/APISERVER_OBSERVED
+# (tasks 6.2-6.4) associative arrays for the live-run path; a synthetic
+# caller builds the same shape by hand, with no deployment involved at all.
+#
+# Per design.md's Error Handling, a REJECTED resource name (Requirement 1.2)
+# is a run-level failure in its own right -- reported as its own failure
+# category ("REJECTED", distinct from a per-layer MISMATCH/UNREACHABLE) and
+# always drives the overall verdict to FAIL, independent of the (nonexistent
+# -- a rejected name is never actually applied anywhere, see
+# query_controller_resvals()/query_scheduler_resvals()/
+# query_apiserver_resvals()'s shared skip rule) per-layer results.
+
+# Record format (9 fields, separated by VERDICT_FIELD_SEP -- ASCII Unit
+# Separator 0x1F, deliberately NOT a tab, see that constant's own comment
+# below), one record string per checked resource:
+#   resource<SEP>injected<SEP>injection_status<SEP>
+#   ctrl_reachable<SEP>ctrl_observed<SEP>
+#   sched_reachable<SEP>sched_observed<SEP>
+#   api_reachable<SEP>api_observed
+#
+#   resource           -- resource name.
+#   injected            -- the value that was (or would have been) injected.
+#   injection_status    -- "accepted" or "rejected: <reason>"
+#                          (INJECTED_RESULTS' own vocabulary, tasks 2.2/6.1)
+#                          -- anything other than the literal string
+#                          "accepted" is treated as a rejection, independent
+#                          of the reason text.
+#   *_reachable          -- "1" if that layer produced an observed value
+#                          within scope/bounded wait, "0" if it did not
+#                          (timeout, or simply never checked). Ignored when
+#                          injection_status != "accepted" -- nothing was ever
+#                          applied anywhere for a rejected name, so no layer
+#                          is "checked" for it at all (matches tasks
+#                          6.2-6.4's own skip rule).
+#   *_observed           -- observed value string for that layer, meaningful
+#                          only when the paired *_reachable flag is "1".
+#
+# A record for a rejected resource still needs all 9 fields present
+# (positional read) -- pass "0"/"" for each of the 6 layer fields by
+# convention (see build_verdict_record() below); their content is ignored.
+#
+# VERDICT_FIELD_SEP: ASCII Unit Separator (0x1F), NOT a tab. Found live
+# (this task's own synthetic sanity-check run, scenario (c) below) that a
+# plain `IFS=$'\t' read` -- the convention every OTHER TSV in this file
+# uses -- silently COLLAPSES a genuinely-empty field: bash treats tab (like
+# space/newline) as "IFS whitespace" for splitting purposes even when IFS is
+# set to ONLY a tab character, so two adjacent delimiters (an empty
+# *_observed field, which legitimately happens both for an UNREACHABLE
+# layer and for a reachable-but-empty-value MISMATCH, e.g. controller "no
+# value" -- see query_controller_resvals()) collapse into one, shifting
+# every field after it and silently corrupting the record. None of this
+# file's other TSVs (injection-results.tsv, *-check-results.tsv) ever carry
+# a genuinely-empty field, which is why this gotcha never surfaced before
+# aggregate_verdict()'s record format, the first place in this script where
+# an empty field is a legitimate, expected value. 0x1F is not "IFS
+# whitespace" and is never expected to appear in a resource name, value, or
+# status string, so it splits/preserves empty fields correctly with a plain
+# `IFS=$VERDICT_FIELD_SEP read`.
+VERDICT_FIELD_SEP=$'\x1f'
+
+build_verdict_record() {
+    # build_verdict_record NAME INJECTED INJ_STATUS CTRL_OK CTRL_OBS SCHED_OK SCHED_OBS API_OK API_OBS
+    # Thin formatting helper so main()'s live-run wiring and any hand-built
+    # synthetic record share one place that defines field order/count -- not
+    # itself part of the "self-contained comparison step" (aggregate_verdict()
+    # is that step); a caller may also just printf/construct the
+    # VERDICT_FIELD_SEP-joined string directly, this exists purely for
+    # readability at call sites.
+    local sep="$VERDICT_FIELD_SEP"
+    printf '%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s' \
+        "$1" "$sep" "$2" "$sep" "$3" "$sep" "$4" "$sep" "$5" "$sep" \
+        "$6" "$sep" "$7" "$sep" "$8" "$sep" "$9"
+}
+
+# aggregate_verdict RECORD...
+# THE self-contained comparison/aggregation step (Requirement 6.2/6.3/6.4).
+# Pure function over its arguments: no network I/O, no file reads, no
+# subprocess -- every classification decision is either a direct field
+# comparison or a call to controller_values_match() (also pure). Safe and
+# deterministic to call with hand-built synthetic records, live-run-derived
+# records, or a mix, with no live deployment required either way.
+#
+# Resets and repopulates these globals on every call (so repeated calls in
+# the same shell/test session -- e.g. one per synthetic scenario -- never
+# see a stale result from a previous invocation):
+#   AGG_RESOURCE_STATUS["<name>"]   = "PASS" | "FAIL"
+#   AGG_FAILURE_LINES[]             = one human-readable "resource=... layer=...
+#                                      status=... ..." string per failing
+#                                      (resource, layer) pair or rejected name
+#   AGG_TOTAL_RESOURCES             = count of records passed in
+#   AGG_MISMATCHES                  = count of (resource, layer) pairs
+#                                      classified MISMATCH
+#   AGG_UNREACHABLE                 = count of (resource, layer) pairs
+#                                      classified UNREACHABLE
+#   AGG_REJECTED                    = count of records whose injection_status
+#                                      was not "accepted"
+#   AGG_RESULT                      = "PASS" | "FAIL"
+#
+# Prints one FAILURE line per failing (resource, layer) pair or rejected
+# name (naming the specific resource + layer, per Requirement 6.3), then
+# exactly one VERDICT line, then exactly one terminal PASS/FAIL oracle line
+# -- xce-test-ha's machine-parseable convention, see SKILL.md.
+#
+# Returns (exit code) 0 when AGG_RESULT=PASS, 1 when AGG_RESULT=FAIL -- so a
+# caller (main(), or a synthetic test) can branch directly on the call
+# itself: `if aggregate_verdict "${records[@]}"; then ... fi`.
+declare -A AGG_RESOURCE_STATUS=()
+AGG_FAILURE_LINES=()
+AGG_TOTAL_RESOURCES=0
+AGG_MISMATCHES=0
+AGG_UNREACHABLE=0
+AGG_REJECTED=0
+AGG_RESULT="PASS"
+
+aggregate_verdict() {
+    AGG_RESOURCE_STATUS=()
+    AGG_FAILURE_LINES=()
+    AGG_TOTAL_RESOURCES=0
+    AGG_MISMATCHES=0
+    AGG_UNREACHABLE=0
+    AGG_REJECTED=0
+    AGG_RESULT="PASS"
+
+    local rec resource injected inj_status
+    local ctrl_ok ctrl_obs sched_ok sched_obs api_ok api_obs
+    local layer ok obs resource_failed
+
+    for rec in "$@"; do
+        IFS="$VERDICT_FIELD_SEP" read -r resource injected inj_status \
+            ctrl_ok ctrl_obs sched_ok sched_obs api_ok api_obs <<< "$rec"
+
+        AGG_TOTAL_RESOURCES=$((AGG_TOTAL_RESOURCES + 1))
+        resource_failed=0
+
+        if [ "$inj_status" != "accepted" ]; then
+            AGG_REJECTED=$((AGG_REJECTED + 1))
+            resource_failed=1
+            AGG_FAILURE_LINES+=("resource=$resource layer=injection status=REJECTED detail=\"$inj_status\"")
+        else
+            for layer in controller scheduler apiserver; do
+                case "$layer" in
+                    controller) ok="$ctrl_ok";  obs="$ctrl_obs"  ;;
+                    scheduler)  ok="$sched_ok"; obs="$sched_obs" ;;
+                    apiserver)  ok="$api_ok";   obs="$api_obs"   ;;
+                esac
+
+                if [ "$ok" != "1" ]; then
+                    AGG_UNREACHABLE=$((AGG_UNREACHABLE + 1))
+                    resource_failed=1
+                    AGG_FAILURE_LINES+=("resource=$resource layer=$layer status=UNREACHABLE injected=$injected observed=<none>")
+                elif ! controller_values_match "$injected" "$obs"; then
+                    AGG_MISMATCHES=$((AGG_MISMATCHES + 1))
+                    resource_failed=1
+                    AGG_FAILURE_LINES+=("resource=$resource layer=$layer status=MISMATCH injected=$injected observed=$obs")
+                fi
+            done
+        fi
+
+        if [ "$resource_failed" -eq 1 ]; then
+            AGG_RESOURCE_STATUS["$resource"]="FAIL"
+            AGG_RESULT="FAIL"
+        else
+            AGG_RESOURCE_STATUS["$resource"]="PASS"
+        fi
+    done
+
+    if [ "$AGG_TOTAL_RESOURCES" -eq 0 ]; then
+        # No resources at all is not a legitimate PASS -- nothing was
+        # actually verified. Structural/degenerate case, not expected from
+        # main()'s own live-run wiring (inject_resources() already fail()s
+        # the run before this point if RESOURCE_ARGS somehow resolved empty
+        # -- DEFAULT_RESOURCE_ARGS is always non-empty), but a synthetic
+        # caller could legitimately probe this edge, so classify it
+        # explicitly rather than silently reporting PASS on zero input.
+        AGG_RESULT="FAIL"
+        AGG_FAILURE_LINES+=("no resources were checked -- nothing to verify")
+    fi
+
+    local line
+    for line in "${AGG_FAILURE_LINES[@]}"; do
+        log "FAILURE: $line"
+    done
+
+    log "VERDICT resources=$AGG_TOTAL_RESOURCES layers_checked=3 mismatches=$AGG_MISMATCHES unreachable=$AGG_UNREACHABLE rejected=$AGG_REJECTED result=$AGG_RESULT"
+
+    if [ "$AGG_RESULT" = "PASS" ]; then
+        log "PASS: $AGG_TOTAL_RESOURCES resource(s), 3 layers checked, zero mismatches, zero unreachable, zero rejected"
+        return 0
+    else
+        log "FAIL: $AGG_TOTAL_RESOURCES resource(s), 3 layers checked, $AGG_MISMATCHES mismatch(es), $AGG_UNREACHABLE unreachable, $AGG_REJECTED rejected -- see FAILURE lines above"
+        return 1
+    fi
+}
+
+# build_verdict_records_from_tsv TSV_PATH
+# Round-2 remediation of the round-1 REJECTED verdict (see the sched_ok/
+# api_ok derivation comment inside for the actual bug/fix). This is main()'s
+# own record-building loop (Task 7.1), extracted verbatim from main() into a
+# standalone function so it is independently callable -- the round-1
+# reviewer's exact finding was that a hand-built-synthetic-record test of
+# aggregate_verdict() alone (task 8.2's harness) could never have caught this
+# bug, because the bug lived entirely in main()'s data-marshaling code
+# (reading $TOP/logs/injection-results.tsv plus the CONTROLLER_OBSERVED/
+# SCHEDULER_OBSERVED/SCHEDULER_CHECK_RESULTS/APISERVER_OBSERVED/
+# APISERVER_CHECK_RESULTS globals into aggregate_verdict()-shaped records),
+# which a bypass-and-call-aggregate_verdict()-directly test never exercises.
+# Extracting it here means a synthetic test can source this file, hand-
+# populate the same globals + a synthetic TSV, call this function directly,
+# and inspect its output -- exercising the EXACT SAME code main() itself
+# runs, not a reimplementation of it. No behavior change from the inline
+# version: same reads, same globals, same build_verdict_record() calls.
+#
+# Populates the global VERDICT_RECORDS array (reset on every call, mirroring
+# aggregate_verdict()'s own reset-globals-on-every-call convention) with one
+# build_verdict_record()-shaped record per line of TSV_PATH. Build one record
+# per INJECTED name, in injection order -- read back from
+# $TOP/logs/injection-results.tsv, the same durable, order-preserving source
+# query_controller_resvals()/query_scheduler_resvals()/
+# query_apiserver_resvals() already read (bash associative arrays have no
+# defined iteration order, the TSV does). ANY_CONTROLLER_MISMATCH/
+# ANY_SCHEDULER_MISMATCH/ANY_SCHEDULER_UNREACHABLE/ANY_APISERVER_MISMATCH/
+# ANY_APISERVER_UNREACHABLE (set by the three query_*_resvals() calls) are
+# NOT consulted here -- aggregate_verdict() does its own independent
+# MATCH/MISMATCH/UNREACHABLE classification straight from the *_OBSERVED
+# values via controller_values_match(), the same pure comparison helper each
+# query_*_resvals() already uses, so a synthetic caller and this live-run
+# path share identical classification logic.
+VERDICT_RECORDS=()
+build_verdict_records_from_tsv() {
+    local verdict_tsv="$1"
+    [ -f "$verdict_tsv" ] || fail "verdict aggregation: $verdict_tsv not found -- inject_resources() must run first"
+
+    VERDICT_RECORDS=()
+    local vname vvalue vstatus
+    local ctrl_ok ctrl_obs sched_ok sched_obs api_ok api_obs
+    while IFS=$'\t' read -r vname vvalue vstatus; do
+        [ -n "$vname" ] || continue
+        if [ "$vstatus" = "accepted" ]; then
+            # ctrl_ok: derived from CONTROLLER_OBSERVED array-key existence.
+            # This is fine as-is (round-2 review confirmed no change needed)
+            # -- query_controller_resvals()/internal_controller_check() has
+            # no separate UNREACHABLE concept at all; a query failure there
+            # aborts the whole run via fail() rather than leaving a
+            # placeholder entry, so "no observed value" legitimately means
+            # MISMATCH for this layer specifically, and array-key existence
+            # is an accurate proxy for "was queried".
+            if [ -n "${CONTROLLER_OBSERVED[$vname]+x}" ]; then
+                ctrl_ok=1; ctrl_obs="${CONTROLLER_OBSERVED[$vname]}"
+            else
+                ctrl_ok=0; ctrl_obs=""
+            fi
+            # sched_ok/api_ok: round-2 fix (remediation of the round-1
+            # REJECTED verdict). query_scheduler_resvals()/
+            # query_apiserver_resvals() (tasks 6.3/6.4) unconditionally set
+            # SCHEDULER_OBSERVED[$vname]/APISERVER_OBSERVED[$vname] for
+            # EVERY accepted+checked name, including on a genuine
+            # UNREACHABLE/timeout -- where the value is just set to "" (see
+            # the `observed=""` branch in both functions). So deriving
+            # sched_ok/api_ok from *_OBSERVED array-key existence (the old,
+            # rejected approach) was ALWAYS 1 for a checked name -- it could
+            # never see the genuine UNREACHABLE status, and aggregate_verdict
+            # fell through to comparing the injected value against an empty
+            # string, misreporting UNREACHABLE as MISMATCH. Fix: derive
+            # sched_ok/api_ok from SCHEDULER_CHECK_RESULTS[$vname]/
+            # APISERVER_CHECK_RESULTS[$vname] instead -- both are always
+            # populated (same unconditional-set guarantee, task 6.3/6.4) with
+            # one of exactly "MATCH", "MISMATCH: injected=... observed=...",
+            # or "UNREACHABLE: ..." (see the declare -A comments above each
+            # array), so a leading "UNREACHABLE:" is the correct, exact
+            # signal -- not the presence/absence of the OBSERVED entry.
+            # The `__UNCHECKED__` fallback below is defensive-only (not
+            # expected to trigger given query_scheduler_resvals()/
+            # query_apiserver_resvals() iterate the exact same accepted-name
+            # list this loop reads from the same injection-results.tsv, so
+            # CHECK_RESULTS is always populated by the time main() reaches
+            # this loop) -- but if it ever DID trigger, treating "never
+            # checked" as ok=1/empty-observed would silently reproduce this
+            # exact bug class, so it's treated as unreachable instead.
+            case "${SCHEDULER_CHECK_RESULTS[$vname]-__UNCHECKED__}" in
+                UNREACHABLE:*|__UNCHECKED__) sched_ok=0; sched_obs="" ;;
+                *) sched_ok=1; sched_obs="${SCHEDULER_OBSERVED[$vname]-}" ;;
+            esac
+            case "${APISERVER_CHECK_RESULTS[$vname]-__UNCHECKED__}" in
+                UNREACHABLE:*|__UNCHECKED__) api_ok=0; api_obs="" ;;
+                *) api_ok=1; api_obs="${APISERVER_OBSERVED[$vname]-}" ;;
+            esac
+        else
+            ctrl_ok=0; ctrl_obs=""
+            sched_ok=0; sched_obs=""
+            api_ok=0; api_obs=""
+        fi
+        VERDICT_RECORDS+=("$(build_verdict_record "$vname" "$vvalue" "$vstatus" \
+            "$ctrl_ok" "$ctrl_obs" "$sched_ok" "$sched_obs" "$api_ok" "$api_obs")")
+    done < "$verdict_tsv"
+}
+
 main() {
    parse_args "$@"
    resolve_top
@ -1158,18 +1506,21 @@ main() {

    query_apiserver_resvals

-    # --- Extension point (task 7.1): aggregate results into a VERDICT line
-    #     plus the terminal PASS/FAIL oracle line, replacing this task's
-    #     placeholder success message and exit code below.
-    #     ANY_CONTROLLER_MISMATCH/ANY_SCHEDULER_MISMATCH/
-    #     ANY_SCHEDULER_UNREACHABLE/ANY_APISERVER_MISMATCH/
-    #     ANY_APISERVER_UNREACHABLE (set by query_controller_resvals()/
-    #     query_scheduler_resvals()/query_apiserver_resvals() above) are
-    #     groundwork for this aggregation -- tasks 6.2/6.3/6.4 themselves
-    #     deliberately do not act on them. ---
+    # --- Task 7.1: aggregate the controller/scheduler/apiserver per-layer
+    #     results plus the injection accept/reject outcome into one VERDICT
+    #     line + terminal PASS/FAIL oracle line (aggregate_verdict() above).
+    #     build_verdict_records_from_tsv() (round-2 extraction, see its own
+    #     header comment) does the data-marshaling; main() is just a thin
+    #     caller. ---
+    build_verdict_records_from_tsv "$TOP/logs/injection-results.tsv"

-    log "PASS (task 5.1+6.1+6.2+6.3+6.4 scope): controller, scheduler, and apiserver confirmed up; worker '$WORKER_HOST' joined and resupdate ran for ${#INJECTED_RESULTS[@]} resource(s) under $TOP (see $TOP/logs/injection-results.tsv for the accept/reject detail per name -- a rejection there is a captured outcome, not a script failure at this stage); controller-side check ran for every accepted resource (see $TOP/logs/controller-check-results.tsv -- a MISMATCH there is also a captured outcome, not a script failure at this stage, see ANY_CONTROLLER_MISMATCH); scheduler-side check ran for every accepted resource (see $TOP/logs/scheduler-check-results.tsv -- a MISMATCH or UNREACHABLE there is also a captured outcome, not a script failure at this stage, see ANY_SCHEDULER_MISMATCH/ANY_SCHEDULER_UNREACHABLE); apiserver-side check ran for every accepted resource (see $TOP/logs/apiserver-check-results.tsv -- a MISMATCH or UNREACHABLE there is also a captured outcome, not a script failure at this stage, see ANY_APISERVER_MISMATCH/ANY_APISERVER_UNREACHABLE); the overall PASS/FAIL verdict lands in task 7.1; cleaning up"
-    exit 0
+    log "aggregating verdict across ${#VERDICT_RECORDS[@]} resource(s) under $TOP; cleanup (trap-based, same mechanism as a mid-deployment abort) fires on exit below regardless of PASS/FAIL, unless --keep-running was passed -- logs under $TOP/logs are preserved either way"
+
+    if aggregate_verdict "${VERDICT_RECORDS[@]}"; then
+        exit 0
+    else
+        exit 1
+    fi
 }

 # Hidden internal entry point (bug-1579 fix): inject_resources()'s `!`
@ -1179,10 +1530,22 @@ main() {
 # (which has no knowledge of this flag and would otherwise reject it as an
 # "Unknown argument") -- runs only internal_controller_check(), none of the
 # normal deploy/inject/trap flow.
-if [ "${1:-}" = "--internal-controller-check" ]; then
-    shift
-    internal_controller_check "$@"
-    exit $?
-fi
+#
+# Guarded so this file can be `source`d (e.g. a synthetic test harness that
+# wants aggregate_verdict()/controller_values_match()/build_verdict_record()
+# in scope, task 8.2's use case) without executing main() or
+# internal_controller_check() as a side effect -- ${BASH_SOURCE[0]} (this
+# file's own path) only equals ${0} when the script is the thing actually
+# being run (directly executed, or `bash run-resource-verify.sh ...`,
+# including the `--internal-controller-check` recursive invocation above,
+# which is a fresh `bash "$SELF_SCRIPT" ...` process, not a source), not
+# when it's sourced into another shell/script.
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    if [ "${1:-}" = "--internal-controller-check" ]; then
+        shift
+        internal_controller_check "$@"
+        exit $?
+    fi

-main "$@"
+    main "$@"
+fi
--- a/.kiro/specs/resource-value-verification/tasks.md
+++ b/.kiro/specs/resource-value-verification/tasks.md
@ -93,7 +93,7 @@
  - _Boundary: xce-test-resources skill (apiserver check) — same file as 6.2/6.3, sequential_

 - [ ] 7. Integration: verdict aggregation, reporting, and cleanup
- [ ] 7.1 Aggregate layer results, emit the verdict, and clean up
+- [x] 7.1 Aggregate layer results, emit the verdict, and clean up
  - Combine the controller/scheduler/apiserver check results into a single per-resource, per-layer comparison, implemented as a self-contained comparison step that accepts a set of (resource, injected value, per-layer observed value) records — so it can be invoked directly with synthetic records for testing (task 8.2), not only reachable from a live run
  - Emit a machine-parseable VERDICT line (following `xce-test-ha`'s convention) plus the standard terminal PASS/FAIL oracle line, naming every layer that mismatched or was unreachable (timed out)
  - Support checking multiple resource names in one run, defaulting to the resource names added in task 2.1 when none are supplied
@ -116,6 +116,7 @@
  - _Depends: 7.1_

 ## Implementation Notes
+- Task 7.1 (round 1 REJECTED, round 2 fixed): `aggregate_verdict()`/`build_verdict_record()` (VERDICT_FIELD_SEP=`$'\x1f'`, ASCII Unit Separator — bash's IFS tab-splitting silently collapses adjacent empty fields even with IFS set to only a tab, bug-1581) are the self-contained, synthetic-testable comparison step task 8.2 needs. `main()`'s live-data wiring is `build_verdict_records_from_tsv()` — derive a layer's "ok" status from `SCHEDULER_CHECK_RESULTS[$name]`/`APISERVER_CHECK_RESULTS[$name]`'s STRING CONTENT (`UNREACHABLE:*` prefix check), never from array-key existence — tasks 6.3/6.4 always set the key, even on a genuine timeout (just with an empty value), so key-existence can never distinguish UNREACHABLE from MISMATCH (bug-1582). `CONTROLLER_OBSERVED`'s key-existence check IS correct for the controller layer specifically (task 6.2 has no separate UNREACHABLE concept — a query failure there aborts the whole run). VERDICT line format: `[xce-test-resources] VERDICT resources=<N> layers_checked=3 mismatches=<M> unreachable=<U> rejected=<R> result=PASS|FAIL` — `rejected=` is an intentional addition beyond design.md's illustrative template (Requirement 6.3 needs rejected names attributable in the machine-parseable line itself); `layers_checked=3` is a fixed constant, not a per-run computed count. SKILL.md fully rewritten to describe the now-complete, functional tool.
 - Task 6.4: `poll_apiserver_log()`/`query_apiserver_resvals()` mirror task 6.3's scheduler-check pattern exactly (same bounded-wait/batched-polling shape). The apiserver's zap `console` encoder renders structured fields as a trailing JSON object (`resource-location stored {"resource": "r15m", "host": "...", "value": 1.5}`), not classic `key=value` text — grep for the message string, then parse the trailing JSON block. All three layers (controller/scheduler/apiserver) confirmed simultaneously MATCH on the full default set including `cpuf`, proving the bug-1579 fix holds end-to-end.
 - **bug-1579 is now FULLY fixed** (retrofit on top of tasks 6.1/6.2, not a numbered task — user-requested follow-up). `inject_resources()`'s worker-context `xce_test` script now inserts a `!<shell-command>` line between `resupdate` and `quit`: `! bash <script> --internal-controller-check <top> <peers> <worker-host> <names...>` — this runs the controller-check query via `xce_test`'s `!` shell-escape (`preprocess_and_run()`, handled in the single top-level REPL loop before context dispatch, so it works in ANY context including `worker` mode without disconnecting). The controller check now runs WHILE the injecting worker's connection is still open, so host-attrs resources are never reset before being checked. `DEFAULT_RESOURCE_ARGS` now includes `cpuf` again (proof the fix works: `cpuf` now MATCHes at the controller layer, not the old false MISMATCH). `query_controller_resvals()`'s comparison logic (`controller_values_match()`) is confirmed byte-for-byte unchanged — this was a pure sequencing fix. Any future task touching `inject_resources()`/the controller-check flow should be aware of this nested-process structure (`--internal-controller-check` is a hidden internal dispatch flag, gated before `main()`/`parse_args()` runs — do not remove it thinking it's dead code).
 - Non-blocking notes from the retrofit's review: `internal_controller_check()` queries ALL requested resource names (including ones that will later be rejected), not just accepted ones like the original design — forced by sequencing (accept/reject isn't known until the whole injecting session's output is parsed after it returns); confirmed benign, the final `controller-check-results.tsv` still correctly omits rejected names. Also: invoking `--internal-controller-check` directly with fewer than 3 args hits `set -u` and dies with a raw "unbound variable" message instead of a clean `fail()` — cosmetic only, this path is unreachable from any normal invocation.