myxce/.kiro/specs/scheduler-controller-msg-gap/research.md

# Research Log: scheduler-controller-msg-gap

> Feature: scheduler-controller-msg-gap
> Created: 2026-05-11

## Discovery Type

Extension (light discovery) — extending existing handler code in an established C codebase.

## Summary

The scheduler's `handler.c` dispatcher processes controller messages via `scheduler_req_handler`. Six message cases are stubbed out or empty-bodied. The `vtask_entry()` assembly layer in `src/lib/xcectx.c` rewrites all `pushmsg` into `datamsg`, making the `Request_body_pushmsg` branch dead code. Existing plugin hooks (`plugin_job_update`, `plugin_queue_update`, `plugin_worker_update`, `plugin_job_state_change`) exist for notifying plugins of state changes. A `plugin_job_rusage_update` hook does not yet exist and must be added. The `src/lib/helper.c` file already provides `xce_request_op_name()`, `xce_request_msg_name()`, and `xce_dataset_table()` — a new `xce_notify_event_name()` function is needed.

## Research Log

### Topic 1: Plugin Hook System

**Source:** `src/scheduler/include/plugin.h`, `src/scheduler/src/plugin.c`

**Findings:**
- Hooks follow a `plugin_*_update(void* sched, xce_sched_*_t*)` pattern
- Called after state mutation to notify registered plugins
- Hooks iterate registered plugins and call their function pointers
- `plugin_job_state_change(void*, xce_sched_job_t*, uint32_t)` takes an extra state parameter
- No `plugin_job_rusage_update` hook exists yet — must be added

**Implication:** New handlers should call the corresponding plugin hook after mutating scheduler state. The rusage handler needs a new hook typedef and global function.

### Topic 2: Object Conversion Functions

**Source:** `src/scheduler/src/schedobj.c`

**Findings:**
- `make_sched_job(void* arg, xce_job_t* j)` — maps jobid, times, state, user/queue/project pointers, resreq, mintasks/maxtasks, allocs, accounts
- `make_sched_queue(void* arg, xce_queue_t* q)` — maps name, attrs, status, priority, accept_interval, sched_delay, resreq
- `make_sched_worker(void* arg, xce_worker_t* w)` — maps name, status, idx, resvals zarray from `w->resvals_to_sched`
- `kill_sched_*` functions free their respective objects

**Implication:** Job/queue/worker update handlers should use `make_sched_*` for create-or-update semantics. For updates (existing objects), we need update-in-place logic since `make_sched_*` always creates new objects.

### Topic 3: Resource and Shared Instance Initialization

**Source:** `src/scheduler/src/scheduler.c`

**Findings:**
- `sched_init_resources()` — iterates `config->resources[]`, calls `makeResource(sched->resmgr, r)` for each, warns on NULL/duplicate
- `sched_init_sharedInst()` — iterates `config->resvals[]`, uses `MOVE_PTR` to steal val pointers into `sched->sharedInst` zarray
- Both called from `sched_init_config()` during initial config push
- For runtime updates, we need incremental versions that can add new or update existing resources

**Implication:** Dataset handlers for `Data_data_resources` and `Data_data_resvals` need new update functions that can both create and modify existing entries.

### Topic 4: Existing Helper Functions

**Source:** `src/lib/helper.c`, `src/include/helper.h`

**Findings:**
- `xce_request_op_name(xce_request_t* req)` — returns string for OPCode enum
- `xce_request_msg_name(xce_request_t* req)` — returns string for Request body tag
- `xce_dataset_table(int table)` — returns string for Data table type enum
- All return `"unknown"` for unrecognized values
- `xce_notify_event_name()` does NOT exist yet — must be created
- Signature pattern: `char* func_name(enumerated_type_or_struct_ptr)` returning `char*`

**Implication:** Create `xce_notify_event_name(int event)` following same pattern. Also consider `char* xce_sched_op_name(xce_sched_op_t op)` for the update request op enum.

### Topic 5: Cap'n Proto Enum Values (NotifyData)

**Source:** `src/schema/controller.capnp`

**Findings:**
All 19 NotifyData event tags:
- `jobPidInfo(0)`, `jobFailInfo(1)`, `jobDispatchFail(2)`, `jobFinishInfo(3)`, `schedStartJob(4)`, `schedExitJob(5)`, `schedFailJob(6)`, `schedUpdateJob(7)`, `schedUpdateResvals(8)`, `stoppedJob(9)`, `resumedJob(10)`, `ckptinfo(11)`, `schedUpdateQueue(12)`, `taskPidInfo(13)`, `taskFailInfo(14)`, `taskFinish(15)`, `postDoneJob(16)`, `schedJobRusages(17)`, `updateJob(18)`

**Implication:** The `xce_notify_event_name()` function must cover all 19 values. Only the scheduler-relevant ones need handler implementations (schedUpdateJob, schedUpdateQueue, schedUpdateResvals, schedJobRusages are the gap items).

### Topic 6: Dead Code Analysis

**Source:** `src/lib/xcectx.c` lines 93-106, `src/scheduler/src/handler.c` lines 172-197

**Findings:**
- `vtask_entry()` accumulates `pushmsg` rows into `ctx->dataset`, then rewrites `body_which = Request_body_datamsg` and `body.datamsg = ctx->dataset`
- The `Request_body_pushmsg` case in `scheduler_req_handler` (lines 185-198) calls `sched_handle_jobpush()` and `sched_handle_workerpush()`, both of which are stubs returning 0
- These functions can never be reached via normal message flow

**Implication:** Safe to remove the pushmsg case branch and both stub functions.

## Architecture Decisions

1. **Handler pattern:** New notify handlers follow the same enqueue-to-pend-updates pattern as `sched_on_job_state_change` — create a `xce_sched_update_request_t`, set the op, enqueue via `enq_pend_update()`, process in `sched_update()` main loop.

2. **Update-in-place for job/queue/worker:** For NotifyData update events, we need update semantics (find existing, mutate fields) rather than create semantics (make_sched_*). New `sched_update_job`, `sched_update_queue`, `sched_update_worker` functions will handle this.

3. **New XCE_SCHED_OP enum values:** Need `XCE_SCHED_OP_UPDATE_RESVALS` for resource value updates and `XCE_SCHED_OP_JOB_RUSAGE` for rusage updates.

4. **Plugin hook for rusage:** Add `plugin_job_rusage_hook_t` typedef and `plugin_job_rusage_update()` global function following existing patterns.

5. **Helper function:** Add `xce_notify_event_name(int)` and `xce_sched_op_name(xce_sched_op_t)` to `src/lib/helper.c` and `src/include/helper.h`.

---

# Cross-Spec Gap Audit (2026-06-08)

> Batch `/kiro-validate-gap` across 9 specs on persistent_ha branch.

## Analysis Summary

- **scheduler-controller-msg-gap** is **~100% complete**. All 9 task groups, 28 subtasks (1.1–9.9) are `[x]` (9.9 optional).
- **No remaining gaps**: All 8 requirements have verified implementation. The 6 previously stubbed notify/dataset handlers (`schedUpdateJob`, `schedUpdateQueue`, `schedUpdateResvals`, `schedJobRusages`, `Data_data_resources`, `Data_data_resvals`) are wired with full business logic. Dead code (`pushmsg` branches, `sched_handle_jobpush`, `sched_handle_workerpush`) removed. Symbolic logging via `xce_notify_event_name()` and `xce_sched_op_name()` is in place.
- **Optional gap**: Task 9.9 (verify WARN log when handler returns non-zero) is marked optional — requires log capture infrastructure not yet present in the test harness.
- **Cross-spec relationship**: `resval-id-decoupling` uses the same pattern (name-based resource lookup via `sched->resmgr`) in `sched_init_sharedInst`. The `XCE_SCHED_OP_UPDATE_RESVALS_FULL` handler added in this spec also resolves by name — consistency is maintained.
- **Recommendation**: The spec can be marked `implemented`. Task 9.9 can be closed as deferred.
---

# Gap Analysis — Validation Pass (2026-06-09)

> Method: per `.claude/skills/kiro-validate-gap/rules/gap-analysis.md`. Every requirement re-verified against current source on `persistent_ha` (handler.c, scheduler.c, schedobj.c, plugin.c, helper.c, resobj.c, controller sched.c, testsuite/test.c). This pass **revises the 2026-06-08 audit's "~100% complete" conclusion**: handler wiring is complete, but four acceptance criteria are only partially met, two latent bugs exist in the dequeue paths, payload memory is leaked for the new op types, and the Task 9 test checkboxes substantially overstate actual coverage.

## 1. Current State Summary

- All 7 controller-emitted NotifyData events (`src/controller/sched.c`: schedStartJob, schedExitJob, schedFailJob, schedUpdateJob, schedJobRusages, schedUpdateResvals, schedUpdateQueue) have non-stub scheduler handlers in `src/scheduler/src/handler.c` (`sched_handle_notify`, lines 275–323). Unknown events return `-EINVAL`.
- Both previously-empty dataset cases (`Data_data_resources`, `Data_data_resvals`) are wired in `sched_handle_dataset` (handler.c:130–144) with ownership transfer (`dataset->nrows = 0; dataset->rows = NULL`).
- Dead `Request_body_pushmsg` branch and `sched_handle_jobpush`/`sched_handle_workerpush` are fully removed; the only remaining reference is a comment in testsuite/test.c:459. Unknown `body_which` returns `-EINVAL` (handler.c:349–351).
- The enqueue→dequeue two-phase pattern is followed: all new handlers enqueue `xce_sched_update_request_t` via `enq_pend_update()`; `sched_update()` (scheduler.c:727–845) processes the five new ops on the main thread.
- Note: no controller code currently **pushes** `Data_data_resources`/`Data_data_resvals` to the scheduler — `src/controller/sched.c` pushes only schedjobs/schedworkers; `src/controller/opget.c` serves these tables only as get-responses. The Req 1 handlers are forward-looking (harmless, but untriggered in production today).

## 2. Requirement-to-Asset Map

| Req / AC | Asset(s) | Status | Notes |
|---|---|---|---|
| 1 / UB-1.1 (resources add **or update**) | handler.c:58–79 `sched_handle_resources`; scheduler.c:770–781 `XCE_SCHED_OP_UPDATE_RESOURCES` | **Partial** | Dequeue calls `makeResource()`, but `makeResource_` (src/lib/resobj.c:108–140) returns NULL with `EEXIST` when the name already exists — **update semantics are missing**; re-pushed catalogs silently fail to modify existing resources. Return value is also ignored (no log), violating the error-handling steering ("never silently swallow errors"). |
| 1 / UB-1.2 (resvals → sharedInst by index) | handler.c:81–102; scheduler.c:782–799 `UPDATE_RESVALS_FULL` | Implemented | Resolves resource by name via `key_find(sched->resmgr,…)`, puts at `r->idx` — consistent with `sched_init_sharedInst` and the resval-id-decoupling spec. Unknown resources WARN-and-skip. |
| 1 / UB-1.3 (skip NULL rows) | scheduler.c:773–775, 786–788 | Implemented | `continue` on NULL entries in both loops. |
| 1 / UB-1.4 (-ENOMEM, free partial) | handler.c calloc guards | Implemented | Handlers return `-ENOMEM`; no partial allocations to free at enqueue level. |
| 2 / UB-2.1, UB-2.2 (dead code gone) | handler.c:325–362 | **Implemented** | `Request_body_pushmsg` case and both stub functions absent from source; `grep` finds no definitions anywhere in src/scheduler/. |
| 3 / UB-3.1 (update mutable fields: state, priority, queue, resreq) | scheduler.c:707–725 `sched_update_existing_job` | **Partial** | Only `job->state` is copied from the update payload. Priority, queue, and resource-request fields named in the requirement are not updated. `plugin_job_update()` is called. |
| 3 / UB-3.2 (-ENOENT unknown jobid) | scheduler.c:716–718 | Implemented | Wait — caveat: `sched_update_existing_job` exists but the `XCE_SCHED_OP_UPDATE_JOB` dequeue case routes to `sched_update_jobs()` (scheduler.c:754–756), which is the bulk create path. Whether a notify-driven single-job update reaches `sched_update_existing_job` needs verification (see Research Needed R-2). |
| 3 / UB-3.3 (-EINVAL NULL payload) | handler.c:180–182 | Implemented | |
| 4 / UB-4.1, 4.2, 4.3 (queue update) | schedobj.c:485–511 `sched_update_existing_queue`; scheduler.c:767–769 | Implemented | status/priority/accept_interval/sched_delay/resreq all updated; resreq freed + reallocated; `-ENOENT` on unknown queue; `-EINVAL` on NULL; `plugin_queue_update()` called. |
| 5 / UB-5.1 (per-worker resvals update) | scheduler.c:801–818 `UPDATE_RESVALS` | **Partial — bug** | `zarray_put(&w->resvals, 0, rv->val)` writes **index 0 unconditionally**. Worker resvals are indexed by resource idx (`zarray_put(&(worker->resvals), r->idx, val)`, schedobj.c:291) and read back via `zarray_get(&w->resvals, i)` in resource.c:161. The handler must resolve `rv->name` → `sched->resmgr` → `r->idx` and put at that index; as written, every update clobbers slot 0 regardless of resource. |
| 5 / UB-5.2 (skip unknown host) | scheduler.c:808–811 | Implemented | `sched_get_worker()` NULL → `continue`. |
| 5 / UB-5.3 (zero values → return 0) | handler.c:235–237 | Implemented | NULL or `count == 0` short-circuits before enqueue. |
| 6 / UB-6.1 (**store** rusage on job + plugin) | scheduler.c:819–831 `JOB_RUSAGE`; plugin.c:361 `plugin_job_rusage_update` | **Partial** | Plugin hook is implemented and called, but the rusage data is **not stored on the job object** as the requirement states — after the plugin call the payload is dropped (and leaked, see §3). |
| 6 / UB-6.2 (-ENOENT unknown jobid) | scheduler.c:822–824 | Implemented | |
| 6 / UB-6.3 (-EINVAL NULL rusage) | handler.c:257–259 | Implemented | |
| 7 / UB-7.1 (WARN with op + errno) | scheduler.c:838–841 | Implemented | `WARN("%s: op %s failed: %s (%d)", …, sched_op_name(req->op), strerror(-rc), rc)` after the switch. |
| 7 / UB-7.2 (unknown notify → -EINVAL) | handler.c:315–317 | Implemented (non-regression confirmed) | |
| 7 / UB-7.3 (unknown body_which → -EINVAL) | handler.c:349–351 | Implemented (non-regression confirmed) | |
| 8 / UB-8.1 (op name in req handler log) | handler.c:330 | Implemented | `TRACE("%s op = %s", __func__, xce_request_op_name(request))`. |
| 8 / UB-8.2 (dataset table name) | handler.c:109 | Implemented | `xce_dataset_table(dataset->table)`. |
| 8 / UB-8.3 (notify event name) | handler.c:279 | Implemented | `xce_notify_event_name(notify->event)`. |
| 8 / UB-8.4 (UNKNOWN(%d) fallback) | src/lib/helper.c:367–415 | Implemented | All 19 NotifyData tags covered + `snprintf(buf, …, "UNKNOWN(%d)", event)`. |
| 8 / UB-8.5 (symbolic op name in WARN) | scheduler.c:685–706 `sched_op_name` | Implemented — **deviation** | Implemented as **static `sched_op_name()` in scheduler.c**, not `xce_sched_op_name()` in `src/lib/helper.c`/`src/include/helper.h` as design Component 8 and tasks 1.1/1.3 (both `[x]`) state. The deviation is arguably correct (the enum is scheduler-internal; libxce cannot see `xce_sched_op_t`), but tasks.md and design.md no longer reflect reality. Uses a non-thread-safe `static char buf[32]` — acceptable since only the main thread calls it; same pattern as the helper. |
| 9 (tests) | src/scheduler/testsuite/test.c | **Partial — checkboxes overstate coverage** | See §4. |

**Memory-ownership constraint (design risk realized):** `sched_handle_notify` nullifies `notify->data.*` after successful enqueue (handler.c:297–315), transferring ownership from `free_Request()` to the update request — but `sched_pend_update_killer` (scheduler.c:311–321) has empty `break;` for all five new ops and the dequeue cases never free the payloads. Result: every `schedUpdateQueue` leaks an `xce_queue_t`, every `schedUpdateResvals` leaks the `xce_resval_list_t` (entries + name/host strings; only `rv->val` survives into the zarray — and on the index-0 bug path even that gets clobbered without a free), every `schedJobRusages` leaks the rusages struct, and every resources/resvals dataset leaks the row array and row structs (`makeInstance` copies; originals are never freed). The `UPDATE_JOB` path is fine (existing killer frees via `free_Job_ptr`). The design's High-severity risk row predicted exactly this; the mitigation was only half-applied (transfer done, release missing).

## 3. Identified Gaps (priority order)

1. **G-1 (bug, correctness): UPDATE_RESVALS writes worker resval index 0 unconditionally** — scheduler.c:813. Breaks scheduling decisions that read per-resource values. (Req 5)
2. **G-2 (bug, correctness): UPDATE_RESOURCES cannot update an existing resource** — `makeResource` → `EEXIST` → NULL, return ignored, no log. "Add or update" is only "add". (Req 1 / UB-1.1)
3. **G-3 (leak): killer/dequeue never free new-op payloads** despite ownership transfer (queue, resval_list, rusages, resource/resval rows). Slow per-message leak in a long-lived daemon. (Design risk #1)
4. **G-4 (requirement shortfall): job update copies only `state`** — priority/queue/resreq not applied. (Req 3 / UB-3.1)
5. **G-5 (requirement shortfall): rusage not stored on job object** — only the plugin hook sees it. Requires a rusage field on `xce_sched_job_t` (none exists today). (Req 6 / UB-6.1)
6. **G-6 (test debt): tasks 9.2, 9.3, 9.4, 9.6, 9.7 are checked `[x]` but absent from test.c.** The only new tests are: 9.1 notify-event-name (real), an enum compile-presence check, 9.5 reduced to a NULL-guard exercised against a **local stub** of `sched_update_existing_queue` defined inside test.c:25–34 (the real schedobj.c implementation is never linked/tested), and 9.8 reduced to `TEST_PASS()` with a comment. `xce_sched_op_name`/`sched_op_name` has no test (9.1 claims it does). None of the enqueue handlers, dequeue cases, or `sched_update_existing_job` are tested.
7. **G-7 (doc drift): design Component 8 / tasks 1.1, 1.3 say `xce_sched_op_name` lives in lib/helper** — it is a static in scheduler.c. Update spec docs or move the function.

## 4. Implementation Approach Options

**Option A — Extend in place (minimal correctness patch).** Fix G-1 (resolve `rv->name` → resmgr idx, mirroring schedobj.c:284–291), G-2 (on `EEXIST` find existing resource and mutate fields, or add an `updateResource()` beside `makeResource()` in resobj.c; log NULL returns), G-3 (add `free_*` calls in `sched_pend_update_killer` per op, reusing the deep-free helpers in src/lib/request.c), G-4 (copy priority/queue/resreq in `sched_update_existing_job`), and G-7 (doc edit). Defer G-5/G-6.
- ✅ Smallest diff, all in files already touched by this spec; restores correctness fast.
- ❌ Leaves UB-6.1 unmet and test checkboxes false; test debt persists.

**Option B — New test program + extracted ops module.** Create `src/scheduler/testsuite/test_msg_handlers.c` (links scheduler objects + generated capnp headers, which test.c cannot include — the stated reason 9.8 became a comment), and optionally extract the five dequeue case bodies into `src/scheduler/src/msgops.c` with non-static functions so they are unit-testable without including scheduler.c.
- ✅ Makes every acceptance criterion genuinely testable; honest Task 9; aligns with testing steering (tiered test_*.c programs already exist in other daemons).
- ❌ Larger diff; new file + meson.build change; refactor risk in scheduler.c.

**Option C — Hybrid (recommended).** Phase 1: Option A bug/leak fixes (G-1…G-4, G-7) with TDD where linkable. Phase 2: Option B test program covering handler enqueue + dequeue paths, replacing the stub-based 9.5 and the TEST_PASS 9.8 with real assertions; un-check or re-implement tasks 9.2–9.8 accordingly. Phase 3 (optional, needs decision): G-5 — add a rusages field to `xce_sched_job_t` or formally amend Req 6 to "deliver to plugins only".
- ✅ Correctness first, honesty in tracking, requirement closure staged.
- ❌ Two-phase coordination; Phase 3 needs a requirements decision.

## 5. Effort & Risk

| Work item | Effort | Risk | Justification |
|---|---|---|---|
| G-1 resvals index fix | S | Low | One-line lookup mirroring existing schedobj.c pattern; resmgr API already used two cases above. |
| G-2 resource update-on-EEXIST | S–M | Medium | Touching shared libxce resobj.c affects controller too; scheduler-side find-and-mutate is the safer S variant. |
| G-3 killer frees | M | Medium | Must trace ownership per type against free_* helpers in request.c; wrong free = double-free under ASAN, wrong no-op = leak persists. |
| G-4 job field updates | S | Low | Copy 3 more fields + string realloc, same shape as queue update. |
| G-5 store rusage on job | M | Medium | New field on `xce_sched_job_t` + lifetime/free in `kill_sched_job`; or amend requirement (zero code). |
| G-6 real test program | M | Medium | New testsuite binary + meson wiring; capnp header dependency is the known friction that derailed Task 9 the first time. |
| G-7 doc sync | S | Low | Edit design.md/tasks.md only. |
| **Total (Option C, Phases 1–2)** | **M (≈4–6 days)** | **Medium** | All within established patterns; risk concentrated in memory-ownership work under ASAN. |

## 6. Recommendations for Design/Implementation Follow-up

1. Treat G-1 and G-2 as **defects against the already-approved spec** — fix before marking the spec `implemented`; both silently corrupt or ignore controller state updates.
2. Adopt **Option C**: in-place fixes first (TDD per project rules — failing test in the new test program first where possible), then the dedicated `test_msg_handlers.c` to make Task 9 honest. Un-check tasks 9.2–9.8 until real tests exist, or annotate them with the stub caveat.
3. Resolve G-3 with the design's own mitigation table: request struct owns the payload after nullification ⇒ `sched_pend_update_killer` must deep-free each new op's payload (reuse `free_Queue_ptr`/`free_Resource_ptr`-style helpers from src/lib/request.c if exported; otherwise add small free helpers).
4. Decide G-5 explicitly: either add the job-side rusage storage Req 6 promises, or amend UB-6.1 — do not leave the requirement and code disagreeing.
5. Sync design.md Component 8 and tasks 1.1/1.3 with the actual `sched_op_name` location (G-7).

## 7. Research Needed

- **R-1:** Which `free_*` deep-free helpers in src/lib/request.c are exported and safe for `xce_queue_t`, `xce_resval_list_t`, `xce_job_rusages_t`, `xce_resource_t*[]` — confirm against ASAN before wiring into the killer.
- **R-2:** Confirm the notify-driven `schedUpdateJob` actually reaches `sched_update_existing_job()`: the dequeue `XCE_SCHED_OP_UPDATE_JOB` case calls `sched_update_jobs()` (bulk path); verify `sched_update_jobs` performs update-in-place for existing jobids (design Component 5 said it should) rather than duplicating jobs.
- **R-3:** Is there a planned controller-side emitter for `Data_data_resources`/`Data_data_resvals` pushes to the scheduler (e.g., post-failover config resync in the persistent_ha work)? If not, decide whether Req 1's handlers stay forward-looking or get an emitter spec.
- **R-4:** Whether `makeInstance(&resobj_type, r)` deep-copies all of `xce_resource_t` (name string included) — determines whether resource row payloads can be freed immediately after `makeResource` returns.

---

## Gap Re-Audit (2026-06-18)

> Post-implementation audit. tasks.md = 36/36 `[x]` (9.9 gated), phase=implemented. Method: every EARS AC re-verified against current on-disk source via codegraph (handler.c, scheduler.c, schedobj.c, plugin.c, helper.c) + grep. This pass tracks the 2026-06-09 G-1..G-7 findings: **G-1 and G-2 are now FIXED; G-3, G-4, G-5, G-6, G-7 remain OPEN; one NEW finding (G-8) supersedes the 2026-06-09 R-2 caveat.**

### 0. Delta vs 2026-06-09 audit

| Prior gap | 2026-06-18 status | Evidence |
|---|---|---|
| G-1 UPDATE_RESVALS clobbered worker index 0 | **FIXED** | scheduler.c:826-831 now resolves `rv->name`→`key_find(sched->resmgr)`→`r->idx`, `zarray_put(&w->resvals, r->idx, rv->val)`; unknown resource WARN+skip. Matches schedobj.c:283-291. |
| G-2 UPDATE_RESOURCES could not update existing resource | **FIXED** | scheduler.c:775-792: on `makeResource()`==NULL (EEXIST), `key_find` the existing resource and copy `valtype/ordtype/flags/interval`; WARN on update and on lookup-miss. "Add-or-update" now satisfied for mutable scalar fields. |
| G-3 killer/dequeue never free new-op payloads (leak) | **OPEN** | `sched_pend_update_killer` (scheduler.c:311-320) still has empty `break;` for UPDATE_QUEUE / UPDATE_RESOURCES / UPDATE_RESVALS_FULL / UPDATE_RESVALS / JOB_RUSAGE. Dequeue cases (scheduler.c:767-847) free nothing. Ownership IS transferred (handler.c:297-316 nullifies `notify->data.*`; sched_handle_dataset:130-145 nullifies rows) ⇒ confirmed per-message leak. |
| G-4 job update copies only `state` | **OPEN (worse — see G-8)** | `sched_update_existing_job` (scheduler.c:707-725) still copies only `job->state`; priority/queue/resreq untouched. AND it is never reached (G-8). |
| G-5 rusage not stored on job | **OPEN** | `xce_sched_job_t` (sched/types.h:80-96) has NO rusage field. JOB_RUSAGE case (scheduler.c:836-847) calls `plugin_job_rusage_update()` then drops+leaks the payload. UB-6.1 "store the usage data on the job object" unmet. |
| G-6 test debt (9.2/9.3/9.4/9.6/9.7 `[x]` but absent; 9.5 stub; 9.8 TEST_PASS) | **OPEN** | testsuite/test.c still defines a LOCAL stub `sched_update_existing_queue` (test.c:25-34) — real schedobj.c impl never linked. Only real new tests: 9.1 notify-event-name (test.c:399-423) + enum compile check. No tests for any enqueue handler, dequeue case, `sched_update_existing_job`, `sched_op_name`, leaks. |
| G-7 doc drift: `xce_sched_op_name` in lib/helper per design/tasks 1.1,1.3 | **OPEN** | Implemented as static `sched_op_name()` in scheduler.c:685-705, NOT `xce_sched_op_name()` in lib/helper.c. design.md Component 8 + tasks.md 1.1/1.3 (both `[x]`) still describe the wrong location/name. |

### 1. NEW finding

- **G-8 (correctness, HIGH — supersedes R-2): `sched_update_existing_job()` is dead code; notify-driven job updates take the bulk CREATE path.** `grep -rn sched_update_existing_job src/` returns ONLY its definition (scheduler.c:707) — zero callers. The `XCE_SCHED_OP_UPDATE_JOB` dequeue case (scheduler.c:755-758) routes to `sched_update_jobs()` (scheduler.c:629-642), which for every job calls `make_sched_job()` (a fresh `calloc`) + `sched_add_job()` (`key_add` into `sched->jobs`). For an existing jobid this either duplicate-keys or replaces the live `xce_sched_job_t` with a new object — discarding `allocs`/`accounts`/`schedobj` runtime state and leaking the old node. There is no `-ENOENT` for an unknown jobid (UB-3.2 unmet) and no in-place mutation (UB-3.1 unmet). Req 3 is effectively NOT closed at the dequeue layer despite the handler-side wiring and the `[x]` on tasks 5.6 / 3.1 / 3.2.

### 2. Requirement-to-Asset map (tagged)

| Req / AC | Asset (file:line) | Tag |
|---|---|---|
| 1 / UB-1.1 resources add-or-update | handler.c:59-80; scheduler.c:770-796 | CLOSED |
| 1 / UB-1.2 resvals→sharedInst by idx | handler.c:82-103; scheduler.c:797-815 | CLOSED |
| 1 / UB-1.3 skip NULL rows | scheduler.c:774, 802-804 | CLOSED |
| 1 / UB-1.4 -ENOMEM, free partial | handler.c:68-71,91-94 | CLOSED |
| 2 / UB-2.1, UB-2.2 dead pushmsg gone | handler.c:333-362 (no pushmsg case); grep: no `sched_handle_jobpush`/`workerpush` | CLOSED |
| 3 / UB-3.1 update mutable fields | scheduler.c:707-725 (`state` only) AND unreachable | OPEN (G-4 + G-8) |
| 3 / UB-3.2 -ENOENT unknown jobid | scheduler.c:716-718 — never reached; live path `sched_update_jobs` create-anyway | OPEN (G-8) |
| 3 / UB-3.3 -EINVAL NULL payload | handler.c:181-183 | CLOSED (enqueue layer) |
| 4 / UB-4.1/4.2/4.3 queue update | schedobj.c:485-511; scheduler.c:767-769 | CLOSED |
| 5 / UB-5.1 per-worker resvals | scheduler.c:816-835 (idx fix) | CLOSED |
| 5 / UB-5.2 skip unknown host | scheduler.c:824-825 | CLOSED |
| 5 / UB-5.3 zero values→0 | handler.c:236-238 | CLOSED |
| 6 / UB-6.1 store rusage on job | scheduler.c:836-847 (plugin only; not stored) | OPEN (G-5) |
| 6 / UB-6.2 -ENOENT unknown jobid | scheduler.c:840-841 | CLOSED |
| 6 / UB-6.3 -EINVAL NULL rusage | handler.c:258-260 | CLOSED |
| 7 / UB-7.1 WARN op+errno | scheduler.c:855-858 | CLOSED |
| 7 / UB-7.2 unknown notify→-EINVAL | handler.c:318-320 | CLOSED |
| 7 / UB-7.3 unknown body_which→-EINVAL | handler.c:350-352 | CLOSED |
| 8 / UB-8.1 op name in req log | handler.c:331 | CLOSED |
| 8 / UB-8.2 dataset table name | handler.c:109 | CLOSED |
| 8 / UB-8.3 notify event name | handler.c:279 | CLOSED |
| 8 / UB-8.4 UNKNOWN(%d) fallback | helper.c (xce_notify_event_name) + scheduler.c:703 | CLOSED |
| 8 / UB-8.5 symbolic op name in WARN | scheduler.c:685-705 `sched_op_name` (static, not lib helper) | CLOSED-with-deviation (G-7) |

**Score: 17 / 23 ACs fully closed.** OPEN: UB-3.1, UB-3.2 (G-8/G-4), UB-6.1 (G-5). Cross-cutting non-AC defects: G-3 (leak), G-6 (test debt), G-7 (doc drift). Of the 8 top-level requirements, **Req 2, 4, 5, 7, 8 are fully CLOSED; Req 1 CLOSED; Req 3 OPEN; Req 6 PARTIAL (delivery yes, storage no).**

### 3. Residual / deferred gaps (priority order)

1. **G-8 (HIGH, correctness)** — notify `schedUpdateJob` hits the create path; `sched_update_existing_job` is dead code. Corrupts/duplicates live job state; no -ENOENT. Req 3 unmet.
2. **G-3 (MED, leak)** — five new ops never freed despite ownership transfer. Unbounded slow leak in a long-lived daemon; aggravated by G-8 (old job node also leaked on overwrite).
3. **G-5 (MED, requirement shortfall)** — rusage never stored on job (no field exists). UB-6.1 unmet.
4. **G-4 (LOW, requirement shortfall)** — even once reachable, job update applies only `state`; priority/queue/resreq dropped.
5. **G-6 (MED, test debt / tracking integrity)** — tasks 9.2/9.3/9.4/9.6/9.7 `[x]` but no tests in test.c; 9.5 tests a local stub, not real schedobj.c; 9.8 is TEST_PASS+comment. Checkboxes overstate coverage; none of the dequeue correctness fixes (G-1/G-2) are tested.
6. **G-7 (LOW, doc drift)** — design.md Component 8 + tasks 1.1/1.3 name `xce_sched_op_name` in lib/helper; reality is static `sched_op_name` in scheduler.c.

### 4. Integration risks

- Req 1 handlers remain forward-looking: no controller emitter pushes `Data_data_resources`/`Data_data_resvals` to the scheduler today (controller/sched.c pushes only schedjobs/schedworkers; opget.c serves them only as get-responses). G-1/G-2 fixes are correct but untriggered in production — regression-prone because untested AND unexercised. (Prior R-3 still open.)
- G-8 + G-3 interact: overwriting a live job via `sched_add_job` leaks the prior node's `schedobj`/`allocs`/`accounts`. Surfaces under ASAN only when a notify update arrives for an already-loaded job — exactly the persistent_ha post-failover resync scenario.

### 5. Options for open work (G-8, G-3, G-5, G-4, G-6, G-7)

**Option A — Minimal correctness patch.**
- G-8: in dequeue `XCE_SCHED_OP_UPDATE_JOB`, branch on `njobs==1 && sched_get_job(jobid)!=NULL` → `sched_update_existing_job()`; else keep `sched_update_jobs()`. OR route the notify-only op to a distinct `XCE_SCHED_OP_*` so the single-job update never enters the bulk path.
- G-4: copy priority/queue/resreq (+resreq realloc) into `sched_update_existing_job`, mirroring `sched_update_existing_queue` (schedobj.c:498-506).
- G-3: add per-op `free_*` in `sched_pend_update_killer` (reuse src/lib/request.c deep-free helpers — confirm exports per prior R-1 against ASAN).
- G-7: edit design.md/tasks.md to record the `sched_op_name` location/name deviation.
- Defer G-5, G-6.
- Effort **S–M**, Risk **Medium** (ownership/free work must pass ASAN; double-free risk).

**Option B — Close everything incl. requirement + tests.**
- All of A, plus G-5 (add `xce_job_rusages_t* rusages` to `xce_sched_job_t` + free in `kill_sched_job` + store in JOB_RUSAGE case) and G-6 (new `testsuite/test_msg_handlers.c` linking scheduler objects + capnp headers — the friction that derailed Task 9 originally — replacing the local-stub 9.5 and TEST_PASS 9.8 with real assertions; un-check 9.2-9.8 until real).
- Effort **M–L (~5-7 days)**, Risk **Medium-High** (new test binary + meson wiring; struct lifetime changes).

**Option C — Hybrid (recommended).**
- Phase 1: A's correctness/leak fixes (G-8, G-4, G-3) with a failing test first where linkable; G-7 doc sync.
- Phase 2: G-6 dedicated test program for enqueue+dequeue paths incl. the G-1/G-2 regressions.
- Phase 3 (needs decision): G-5 — add job-side rusage storage, OR formally amend UB-6.1 to "deliver to plugins only" (zero code). Do not leave requirement and code disagreeing.
- Effort **M**, Risk **Medium**; staged so correctness lands first and tracking becomes honest before requirement closure.

### 6. Recommendation

Do NOT treat the spec as truly `implemented`. Five top-level requirements (2,4,5,7,8) and Req 1 are genuinely closed; **Req 3 is broken at the dequeue layer (G-8) and Req 6 stores nothing (G-5)**. Adopt Option C. Re-open/annotate tasks 5.6 (job update wiring), 9.2-9.8 (test debt), and 1.1/1.3 (doc drift). G-8 and G-3 are defects against the approved spec and should block a clean `implemented` sign-off.

### 7. Research still needed

- **R-1 (carry-over):** Confirm exported `free_*` deep-free helpers in src/lib/request.c for `xce_queue_t` / `xce_resval_list_t` / `xce_job_rusages_t` / `xce_resource_t*[]` and validate under ASAN before wiring the killer (G-3).
- **R-3 (carry-over):** Planned controller emitter for `Data_data_resources`/`Data_data_resvals` to scheduler (persistent_ha resync)? Determines whether Req 1 stays forward-looking.
- **R-4 (carry-over):** Does `makeResource`/`makeInstance` deep-copy `xce_resource_t` (name incl.)? Determines if resource rows can be freed right after enqueue (G-3 scope).