docs(worker-rejoin-resync): correct design.md's NULL-alloc encode claim

Found live during task 2.2 implementation (bug-1746): design.md claimed
persist_submit_job_dispatch_fact's NULL-alloc case was "explicit,
tested, backward-compatible" on both apply seams -- true for the DECODE
side (apply_committed_entry/apply_wal_entry, bug-1678's actual fix
target), but the ENCODE side (capnp_encode_joballoc) was never
exercised with NULL before this feature and had a latent unconditional
NULL dereference. Corrected all references to distinguish encode vs.
decode safety and note the bug-1746 fix task 2.2 required.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku

.kiro/specs/worker-rejoin-resync/design.md

@@ -41,7 +41,7 @@ On every worker (re)join — a fresh network reconnect after a disconnect, or a
 - Delta/session-token rejoin optimization.
 - `commit-batch-window`'s commit-urgency tuning — no dependency, no reference.
 - Recovering `job->allocs` (the scheduler's resource allocation) for a job reconciled via Requirement 4.8 that the controller never held a dispatch record for — the data was never known to this controller and cannot be reconstructed from the worker's report. Converges via the existing reserved/actual accounting policy on the job's next rusage report (see Data Models), same as any legitimately-unreserved job today.
-- A parallel WAL fact/decode path for the reconcile-assign case: bug-1678 (fixed 2026-08-25, predates this design) already gave `WAL_JOB_DISPATCH` a `persist_submit_job_dispatch_fact(persist_t*, jobid, state, ts, alloc)` primitive with both apply seams (`apply_committed_entry`, `apply_wal_entry`) decoding an *optional* trailing `JobAlloc` — a `NULL` alloc is explicitly the documented, tested backward-compatible case. `record_job_reconcile_assign_fact` calls this primitive directly with `alloc=NULL` (see Data Models) rather than inventing a second payload shape — out of boundary is building anything new here, not reusing what already exists.
+- A parallel WAL fact/decode path for the reconcile-assign case: bug-1678 (fixed 2026-08-25, predates this design) already gave `WAL_JOB_DISPATCH` a `persist_submit_job_dispatch_fact(persist_t*, jobid, state, ts, alloc)` primitive with both apply seams (`apply_committed_entry`, `apply_wal_entry`) decoding an *optional* trailing `JobAlloc` on the **decode** side, and `record_job_reconcile_assign_fact` calls this primitive directly with `alloc=NULL` (see Data Models) rather than inventing a second payload shape — out of boundary is building anything new here, not reusing what already exists. **Correction (2026-09-01, found live during task 2.2 implementation, bug-1746):** the *encode* side of "NULL alloc" was never actually exercised or tested before this feature — `dispatch_a_job_` (bug-1678's only caller) always passes a real, non-NULL allocation, so a latent NULL-pointer dereference in `capnp_encode_joballoc()` (`persist.c`, calls the unsafe `encode_JobAlloc()` instead of the already-existing NULL-safe `encode_JobAlloc_ptr()` used elsewhere in the same generated file) was never triggered until this feature's `record_job_reconcile_assign_fact` became the first genuinely-NULL caller. Fixed as a minimal guard in `capnp_encode_joballoc` (produces a true zero-length buffer for NULL input, matching exactly what the decode side's `if (alloc_len > 0)` check already expects) — in boundary for task 2.2, since it's the direct, minimal companion fix blocking this call, not new machinery.
 - Extending `xce_join()`/`xcecli_join()`'s own blocking wait (`cbuf_deadline_10s`, `xceapi.c`) to also cover config push or job-state reconciliation. That call is a shared, role-agnostic API (worker, apiserver, scheduler, and `xce_test`/`xceadmin` clients all use it) and already returns as soon as the join reply arrives — well before `on_worker_join` (config push) even runs, unchanged by this feature. Reconciliation has its own, independent, server-side timeout (`resync.c`'s `delaymgr` trigger against `workerResyncTimeoutMs`) and is not gated on, or gating of, the join RPC's own deadline in any way.
 
 ### Allowed Dependencies
@@ -297,7 +297,7 @@ sequenceDiagram
 Both new functions follow `dispatch_a_job_`'s own established pattern (job.c:1686-1748: eager, synchronous mutation under the write lock, immediately followed by a durable append, immediately followed by an explicit `persist_notify_job_updated_already_persisted(c, job)` call) — **not** `record_job_finish_fact`'s purely-deferred pattern (state mutation withheld entirely until `apply_committed_entry` applies the committed fact). This choice is deliberate, not copied without thought: `persist_notify_job_updated_already_persisted`'s own doc comment (`persist.h`) frames it generically ("e.g. `dispatch_a_job_()`'s own... call" — an example, not an exclusive binding) as the sanctioned way to notify for *any* caller whose "live, synchronous state... mutation" would otherwise leave `apply_committed_entry`'s later replay finding "state already matches" and silently skipping its own notify (bug-1679's exact failure mode). Both reconciliation outcomes share dispatch_a_job_'s justification for going eager rather than deferred: the fact being recorded is something the *worker* already confirmed really happened (or definitely didn't) — ground truth already changed outside this controller — not a same-node admin command whose commit could still be rolled back with nothing external yet depending on it (that second, riskier shape is `job_ctrl`'s `mark_job_update`, explicitly flagged in `roadmap.md`'s backlog as a legacy pattern to migrate *away* from, not a template to extend). Mixing an eager mutation for one field (e.g. `allocs`) with a deferred mutation for another (`state`) on the *same* transition was considered and rejected — it would create a real window where a job reads back as `RUN` with `allocs == NULL`, an inconsistent combination nothing else in this codebase produces; both fields move together, eagerly, under one lock hold.
 
 - `record_job_reconcile_pending_fact(cluster_t*, job_t*)`: under the job's write lock, calls `apply_job_update_transition(job, JOB_STAT_PEND)` directly (not as a "peek" — as the real, live mutation) — this reuses the existing `job_state_is_backward` guard unchanged, returning early as a no-op if it reports `JOB_TRANSITION_REJECTED_BACKWARD` or `JOB_TRANSITION_NOOP_DUPLICATE` (Requirement 4.3/4.4). Only on `JOB_TRANSITION_APPLIED`: releases `job->job.allocs` (`free_JobAlloc_ptr(&job->job.allocs); job->job.allocs = NULL;`) if non-NULL, still under the same lock hold — this is required, not optional: `apply_job_update_transition` only ever touches `job->job.state` (job.c:2808-2834, confirmed by reading it) and never allocs, and bug-1631 (confirmed live, deliberately left unfixed) documents exactly this gap on `dispatch_a_job_`'s own retryable-failure paths, scoped as unreachable "only because nothing in the current system re-admits an already-RUN job back into the scheduler's PEND pool." Requirement 4.6 is precisely that re-admission path. Then, still under the lock: durably appends the fact via the existing generic `persist_submit_job_fact(&c->persist, WAL_JOB_UPDATE, jobid, JOB_STAT_PEND)` (the plain `{jobid, state}` shape is sufficient here — no extra fields, unlike DISPATCH/START/FINISH's richer siblings). After unlocking: calls `persist_notify_job_updated_already_persisted(c, job)`, reaching the scheduler through Requirement 7's existing notify path exactly as `dispatch_a_job_` already does for its own eager mutation. Also used for Requirement 5.4 (dispatch-to-disconnected-worker fail-fast) — see the `job.c` split below; the allocs release is a no-op there since a disconnected-worker dispatch attempt never committed one.
-- `record_job_reconcile_assign_fact(cluster_t*, job_t*, worker_t*, xce_jobstate_report_t*)`: under the job's write lock, eagerly sets `job->job.state` and the host assignment to match the worker's report (mirroring `dispatch_a_job_`'s own inline mutation at job.c:1686-1717, minus the allocation fields — there are none to set); `job->job.allocs` stays `NULL` (see Data Models — there genuinely is none to write). Then, still under the lock: calls the existing `persist_submit_job_dispatch_fact(&c->persist, jobid, state, ts, alloc)` (job.c:1731-1734, the exact primitive `dispatch_a_job_`'s own success path already calls) directly, with `alloc = NULL` — bug-1678's fix (2026-08-25) already made this primitive's NULL-alloc case an explicit, tested, backward-compatible branch on both apply seams, so this is a thin caller into existing, proven code, not a new WAL fact shape. After unlocking: calls `persist_notify_job_updated_already_persisted(c, job)`, same as above and for the same reason — without it, `apply_committed_entry`'s later replay of this exact `WAL_JOB_DISPATCH` entry finds state already matching and silently skips its own notify, reproducing bug-1679 at this new call site if omitted.
+- `record_job_reconcile_assign_fact(cluster_t*, job_t*, worker_t*, xce_jobstate_report_t*)`: under the job's write lock, eagerly sets `job->job.state` and the host assignment to match the worker's report (mirroring `dispatch_a_job_`'s own inline mutation at job.c:1686-1717, minus the allocation fields — there are none to set); `job->job.allocs` stays `NULL` (see Data Models — there genuinely is none to write). Then, still under the lock: calls the existing `persist_submit_job_dispatch_fact(&c->persist, jobid, state, ts, alloc)` (job.c:1731-1734, the exact primitive `dispatch_a_job_`'s own success path already calls) directly, with `alloc = NULL` — bug-1678's fix (2026-08-25) already made this primitive's NULL-alloc case an explicit, backward-compatible branch on both **apply/decode** seams; the **encode** side had a latent NULL-deref (never exercised until this task — bug-1746, fixed as part of task 2.2, a minimal guard in `capnp_encode_joballoc`), so this is a thin caller into existing code that this task also had to make genuinely NULL-safe, not a new WAL fact shape. After unlocking: calls `persist_notify_job_updated_already_persisted(c, job)`, same as above and for the same reason — without it, `apply_committed_entry`'s later replay of this exact `WAL_JOB_DISPATCH` entry finds state already matching and silently skips its own notify, reproducing bug-1679 at this new call site if omitted.
 - Split in `dispatch_a_job_` (job.c:1511-1512): `w->ep == NULL` is checked and resolved **before** the existing `w->host.status != 0` check reaches `xce_classify_dispatch_fail()` at all — calling `record_job_reconcile_pending_fact()` directly, with the job's `dispatch_fail_count` left untouched and `mark_job_fail()` never called, then returning. This is deliberately a bypass, not a third value added to `xce_dispatch_fail_class_t`: that enum's whole contract (per its own precedent, `commit-batch-window`'s requirements.md: "an exhaustive site-identity enum maps to a decision — TERMINAL vs RETRYABLE") is a two-way choice every other call site's `switch` already exhaustively handles; adding a third value would force each of those ~5 unrelated switches to grow a case they can never hit. `w->host.status != 0` (any bit, `XCE_WORKER_RECONCILING` included) is otherwise unchanged — still reaches `xce_classify_dispatch_fail(XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY)` and its existing retryable/terminal path, same as today, on the (now rare, since the scheduler-side filter keeps it from being the common case) chance a dispatch decision predates the bit being observed.
 - No function introduces a new *commit/idempotency* guard — all three reuse existing guarded seams (`apply_job_update_transition`'s `job_state_is_backward`, `persist_submit_job_fact`/`persist_submit_job_dispatch_fact`'s existing WAL append contracts) and the existing, already-proven-safe `persist_notify_job_updated_already_persisted` notify primitive. Nothing here is a new persistence *mechanism* — it is the same eager-mutation-plus-explicit-notify shape `dispatch_a_job_` already established, applied to two new call sites.
 
@@ -420,9 +420,9 @@ Per `design-principles.md`'s Persistence Review requirement and `persistence.md`
 | Finished failure | `record_job_fail_fact` (existing, reused as-is) | None — same Requirement 4.2 worker-reported-time rule applies | Unchanged, existing |
 | Still running | No fact — pure no-op against the outstanding set | N/A | N/A |
 | Unrecognized → pending | `persist_submit_job_fact(..., WAL_JOB_UPDATE, jobid, JOB_STAT_PEND)` (existing generic primitive, new call site) | None — the plain `{jobid, state}` shape is sufficient, same as every other `WAL_JOB_UPDATE` caller. **`job->job.allocs` release is in-memory only, eager, not itself a persisted field** — mutated live under the same lock as the state change (mirroring `dispatch_a_job_`'s own success path in reverse; see Components), then notified via `persist_notify_job_updated_already_persisted`, not left for `apply_committed_entry` to discover | Both seams already decode plain `WAL_JOB_UPDATE` identically today (it is not a new payload shape) — a replay of this fact re-derives `job->job.state = JOB_STAT_PEND` correctly; it does **not** re-derive the allocs release (that mutation is not wire content), but since `dispatch_a_job_`'s eager pattern already established this exact asymmetry (live mutation + explicit notify now, replay-idempotent state field only, no replay-time side effect needed for a purely in-memory cleanup), this is consistent with existing precedent, not a new gap |
-| Unqueried job → assign | `persist_submit_job_dispatch_fact(..., alloc=NULL)` (existing primitive, new call site, `record_job_reconcile_assign_fact`) | None — reuses `persist_submit_job_dispatch_fact`'s existing `WAL_JOB_DISPATCH` payload shape verbatim with `alloc=NULL`, exercising the same NULL-alloc branch bug-1678 already added and tested. Live mutation is eager (see Components), matching `dispatch_a_job_`'s own pattern for the same fact type | Already covered — both seams (`apply_committed_entry`, `apply_wal_entry`) were updated for this exact NULL-alloc case by bug-1678's fix; no new decode path to verify |
+| Unqueried job → assign | `persist_submit_job_dispatch_fact(..., alloc=NULL)` (existing primitive, new call site, `record_job_reconcile_assign_fact`) | None — reuses `persist_submit_job_dispatch_fact`'s existing `WAL_JOB_DISPATCH` payload shape verbatim with `alloc=NULL`, exercising the same NULL-alloc decode branch bug-1678 already added. Live mutation is eager (see Components), matching `dispatch_a_job_`'s own pattern for the same fact type | Decode side already covered by bug-1678 on both seams. **Encode side required a fix** (bug-1746, task 2.2): `capnp_encode_joballoc` unconditionally dereferenced a NULL `alloc` — this was this codebase's first genuinely-NULL caller. Fixed with a minimal zero-length-buffer guard, matching the decode side's `alloc_len > 0` contract exactly; no new decode path needed once the encode side actually produces the zero-length payload the decode side already expected |
 
-**Documented scope decision — `job->allocs` on a reconciliation-assigned job (Requirement 4.8):** when the controller reconciles a job it never held any prior record of (e.g. it was dispatched by a controller instance before a failover whose dispatch fact was itself lost), it has no way to learn what resource allocation the scheduler originally computed for that job — the worker's report carries the job's execution outcome, not its original resource request. This design deliberately does not attempt to reconstruct `job->allocs` for this case. **Correction (2026-09-01, re-validated against current code):** an earlier draft of this section cited `persistence.md`'s `APPLY_DIVERGENT` section / bug-1678 as evidence this is "a tracked, separate gap" `WAL_JOB_DISPATCH` still has — that citation was stale. Bug-1678 was fixed 2026-08-25 (before this design was finalized): `WAL_JOB_DISPATCH` now durably carries `job->allocs` when one exists, on both apply seams, with an explicit, tested `NULL`-alloc backward-compatible case. The real reason this design still writes no allocation for Requirement 4.8 is simpler and unrelated to that now-closed gap: there just isn't one to write — the scheduler, not this controller, is the only party that ever computes a job's allocation, and for a job this controller never had any record of, no dispatch (successful or otherwise) ever ran through this controller to produce one. `record_job_reconcile_assign_fact` reuses the *fixed* `persist_submit_job_dispatch_fact` primitive precisely because its NULL-alloc case is already the correct, tested representation of "state and host known, allocation not." The practical consequence is bounded by `resource-accounting.md`'s existing policy: a job with an unknown/zero reservation is charged `max(reserved=0, actual)` — i.e., its real usage — the moment its next rusage tick arrives, identical to how a legitimately-unreserved job is already handled today. The job's *state* is correct immediately; its resource *accounting* converges within one rusage cycle, not instantly. No fabricated allocation value is ever written.
+**Documented scope decision — `job->allocs` on a reconciliation-assigned job (Requirement 4.8):** when the controller reconciles a job it never held any prior record of (e.g. it was dispatched by a controller instance before a failover whose dispatch fact was itself lost), it has no way to learn what resource allocation the scheduler originally computed for that job — the worker's report carries the job's execution outcome, not its original resource request. This design deliberately does not attempt to reconstruct `job->allocs` for this case. **Correction (2026-09-01, re-validated against current code):** an earlier draft of this section cited `persistence.md`'s `APPLY_DIVERGENT` section / bug-1678 as evidence this is "a tracked, separate gap" `WAL_JOB_DISPATCH` still has — that citation was stale. Bug-1678 was fixed 2026-08-25 (before this design was finalized): `WAL_JOB_DISPATCH` now durably carries `job->allocs` when one exists, on both apply/decode seams. The real reason this design still writes no allocation for Requirement 4.8 is simpler and unrelated to that gap: there just isn't one to write — the scheduler, not this controller, is the only party that ever computes a job's allocation, and for a job this controller never had any record of, no dispatch (successful or otherwise) ever ran through this controller to produce one. `record_job_reconcile_assign_fact` reuses the *fixed* `persist_submit_job_dispatch_fact` primitive because its NULL-alloc *decode* case is already the correct representation of "state and host known, allocation not" — though the *encode* side of that same NULL case had never actually been exercised before this feature (bug-1746: `capnp_encode_joballoc` dereferenced a NULL `alloc` unconditionally, since `dispatch_a_job_`, bug-1678's only prior caller, always passes a real allocation). Task 2.2 fixed the encode side with a minimal zero-length-buffer guard as part of making this call safe to actually make — a real, if narrow, gap in the "already-tested" characterization this design originally gave the primitive, closed as part of implementation rather than left latent. The practical consequence is bounded by `resource-accounting.md`'s existing policy: a job with an unknown/zero reservation is charged `max(reserved=0, actual)` — i.e., its real usage — the moment its next rusage tick arrives, identical to how a legitimately-unreserved job is already handled today. The job's *state* is correct immediately; its resource *accounting* converges within one rusage cycle, not instantly. No fabricated allocation value is ever written.
 
 ### Configuration Model