test(job-array-single-wal-record): end-to-end integration test for the compact push

Cross-daemon integration test, test-only, spanning the scheduler (C)
and API-server (Go/CGO) receive paths built in tasks 5.1-5.4/5.3b.

Drives real xce_encode()/xce_decode() wire bytes through the actual
production dispatch chain on both daemons -- extract_data() ->
scheduler_req_handler() -> sched_handle_dataset() ->
sched_on_job_array() and go_push_handler() -> handleDataset() ->
cJobArrayToModels()/dispatchJobArray() -- skipping only vtask_entry()'s
~10 lines of static glue, mirrored verbatim in test code (the
if (callnext) treats-any-nonzero-as-proceed behavior that caused
5.3b's bug is reproduced exactly).

Redelivery is tested via genuinely independent fresh decodes of
separately-encoded bytes, not a decode-once-call-twice shortcut.
Bug-catching capability proven by temporarily reverting task 5.3b's
fix and confirming both the C and Go tests fail meaningfully, then
restoring to green -- confirmed independently by both the implementer
and reviewer.

Task 5.5 of job-array-single-wal-record (Requirements 6.1-6.5, 8.4).
This completes Phase 5 (Compact Scheduler/API-Server Push).

src/frontend/apiserver/backend/wire_dispatch_support.go

@@ -0,0 +1,193 @@
+//go:build cgo
+
+package backend
+
+/*
+#include <stdlib.h>
+#include <stdint.h>
+#include "client.h"
+#include "netio.h"
+#include "request.h"
+#include "controller.capnp.h"
+
+// job-array-single-wal-record task 5.5: xce_data_t.data and xce_request_t.body
+// are C unions -- cgo represents these as opaque byte arrays, not named
+// fields, so Go code cannot write/read them directly. These test-support
+// helpers mirror the established pattern this package already uses for the
+// SAME problem (clib.go's jobctrl_set_siginfo/jobctrl_set_ckptoption,
+// val_helpers.h's xce_val_t accessors) -- new test-support C code, no
+// production file touched.
+static void wire_test_data_set_job_array(xce_data_t *d, xce_job_submit_t *ja) {
+    d->data.job_array = ja;
+}
+static void wire_test_request_set_pushmsg(xce_request_t *r, xce_data_t *d) {
+    r->body_which = Request_body_pushmsg;
+    r->body.pushmsg = d;
+}
+static xce_data_t* wire_test_request_get_pushmsg(xce_request_t *r) {
+    return r->body.pushmsg;
+}
+static void wire_test_request_set_datamsg(xce_request_t *r, dataset_t *ds) {
+    r->body_which = Request_body_datamsg;
+    r->body.datamsg = ds;
+}
+*/
+import "C"
+
+import (
+	"fmt"
+	"unsafe"
+)
+
+// wire_dispatch_support.go backs wire_dispatch_test.go (job-array-single-
+// wal-record task 5.5). It is NOT named *_test.go despite being test-only in
+// purpose: the Go toolchain hard-rejects `import "C"` in any file whose name
+// ends in _test.go ("use of cgo in test ... not supported", golang/go#13938
+// -- confirmed against this repo's actual go toolchain source,
+// cmd/go/internal/modindex/read.go). Every function here is unexported and
+// never called by production code (RegisterToBackend, SubmitJob, etc.) --
+// it compiles into the production apiserver binary whenever CGO is enabled
+// (same as any other file in this package) but is inert dead code there,
+// the standard accepted trade-off for driving a real CGO struct through a
+// Go test. wire_dispatch_test.go itself imports nothing from "C" -- it only
+// calls the plain-Go-signature wrappers below.
+
+// wireDispatchBuildPush constructs the exact wire message
+// apiserver_push_jobarray() (apiserver.c, task 5.2) sends -- OPCode_push /
+// Request_body_pushmsg carrying one Data.jobArray descriptor -- and encodes
+// it via the REAL C.xce_encode(), mirroring the C-side test's
+// build_encoded_push() (test_job_array_wire_e2e.c, task 5.5) and
+// test_request.c's test_extract_data_job_array_passes_through_intact (task
+// 5.3b). Returns an opaque handle to the encoded io_packet_t (owned by the
+// caller until wireDispatchDeliver consumes it) usable only with
+// wireDispatchDeliver.
+func wireDispatchBuildPush(baseJobID, arraySize uint64) (unsafe.Pointer, error) {
+	submit := (*C.job_submit_data_t)(C.calloc(1, C.size_t(unsafe.Sizeof(C.job_submit_data_t{}))))
+	submit.queue = C.CString("normal")
+	submit.user = C.CString("tester")
+	submit.resreq = C.CString("type == local")
+	submit.mintasks = 1
+	submit.maxtasks = 1
+
+	jobArray := (*C.xce_job_submit_t)(C.calloc(1, C.size_t(unsafe.Sizeof(C.xce_job_submit_t{}))))
+	jobArray.jobid = C.uint64_t(baseJobID)
+	jobArray.array_size = C.uint64_t(arraySize)
+	jobArray.submit = submit
+
+	data := (*C.xce_data_t)(C.calloc(1, C.size_t(unsafe.Sizeof(C.xce_data_t{}))))
+	data.table = C.Data_data_jobArray
+	C.wire_test_data_set_job_array(data, jobArray)
+
+	var request C.xce_request_t
+	request.opcode = C.OPCode_push
+	C.wire_test_request_set_pushmsg(&request, data)
+
+	pkt := (*C.io_packet_t)(C.calloc(1, C.size_t(unsafe.Sizeof(C.io_packet_t{}))))
+	if rc := C.xce_encode(&request, pkt); rc != 1 {
+		C.free(unsafe.Pointer(pkt))
+		C.free(unsafe.Pointer(submit.queue))
+		C.free(unsafe.Pointer(submit.user))
+		C.free(unsafe.Pointer(submit.resreq))
+		C.free(unsafe.Pointer(submit))
+		C.free(unsafe.Pointer(jobArray))
+		C.free(unsafe.Pointer(data))
+		return nil, fmt.Errorf("xce_encode failed: rc=%d", int(rc))
+	}
+
+	// The hand-built C structs are no longer needed once encoded into pkt's
+	// wire bytes -- xce_decode() will build entirely fresh copies from that
+	// buffer. Free them now so this test doesn't leak.
+	C.free(unsafe.Pointer(submit.queue))
+	C.free(unsafe.Pointer(submit.user))
+	C.free(unsafe.Pointer(submit.resreq))
+	C.free(unsafe.Pointer(submit))
+	C.free(unsafe.Pointer(jobArray))
+	C.free(unsafe.Pointer(data))
+
+	return unsafe.Pointer(pkt), nil
+}
+
+// wireDispatchDeliver mirrors vtask_entry()'s (xcectx.c, static, not
+// exported) push-handling glue: decode -> extract_data() (task 5.3b's real
+// fix) -> reassemble into Request_body_datamsg exactly as vtask_entry()'s
+// MOVE_PTR does -> call the REAL go_push_handler() (clib.go), the top-level
+// CGO entry point xce_backend_handler calls from C, never exercised by any
+// prior apiserver test.
+//
+// ds is allocated via C.calloc (C heap), never a Go stack/heap value,
+// because it is embedded into decoded.body.datamsg and decoded is then
+// handed to C -- storing a Go pointer inside Go memory passed to C violates
+// cgo's pointer-passing rules and panics under the (default-enabled) cgo
+// pointer checker.
+func wireDispatchDeliver(pktHandle unsafe.Pointer) (int, error) {
+	pkt := (*C.io_packet_t)(pktHandle)
+
+	var decoded C.xce_request_t
+	if rc := C.xce_decode(pkt, &decoded); rc != 0 {
+		return 0, fmt.Errorf("xce_decode failed: rc=%d", int(rc))
+	}
+	if decoded.body_which != C.Request_body_pushmsg {
+		return 0, fmt.Errorf("decoded body_which = %d, want Request_body_pushmsg", decoded.body_which)
+	}
+
+	pushmsg := C.wire_test_request_get_pushmsg(&decoded)
+	if pushmsg == nil {
+		return 0, fmt.Errorf("decoded pushmsg is nil")
+	}
+
+	ds := (*C.dataset_t)(C.calloc(1, C.size_t(unsafe.Sizeof(C.dataset_t{}))))
+	ds.table = pushmsg.table
+
+	callnext := C.extract_data(ds, pushmsg)
+	if callnext == 0 {
+		C.free(unsafe.Pointer(ds))
+		return 0, fmt.Errorf("extract_data() returned 0 (reassembly incomplete) -- test harness broken, not exercising the bug class")
+	}
+
+	// FREEUP(decoded.body.pushmsg) equivalent: extract_data() already
+	// transferred/nulled the payload pointer inside pushmsg, so this is a
+	// safe shallow free of the now-hollow xce_data_t container -- mirrors
+	// vtask_entry() (xcectx.c) exactly.
+	C.free(unsafe.Pointer(pushmsg))
+	C.wire_test_request_set_datamsg(&decoded, ds)
+
+	// go_push_handler is defined as a plain (albeit //export-annotated) Go
+	// function in clib.go, in the SAME package -- called directly as a
+	// normal same-package Go function, not via C.go_push_handler (it has no
+	// extern declaration in this file's own cgo preamble; cgo symbol
+	// resolution is per-file, so C.go_push_handler here would fail to
+	// resolve even though both files are in the same package and both carry
+	// //go:build cgo). *C.xce_request_t from this file's preamble is the
+	// same underlying C type as clib.go's parameter (both derive from the
+	// same request.h/controller.capnp.h), so passing &decoded across the
+	// file boundary is type-correct.
+	// Capture ds.rows BEFORE calling go_push_handler: its internal
+	// `defer C.free_Request(req)` runs free_Request(&decoded), whose
+	// Request_body_datamsg case calls free_Dataset_ptr(&decoded.body.datamsg)
+	// -- free_Dataset_ptr() unconditionally free()s the dataset_t pointer it
+	// is given (confirmed by reading controller.capnp.c's generated bodies),
+	// even though free_Dataset() ITSELF is an empty no-op (never touches
+	// ->rows). So `ds` (the container) is freed automatically the instant
+	// go_push_handler returns -- ds is a dangling pointer after that call.
+	// Reading ds.rows afterward would be a use-after-free; separately
+	// free()-ing ds itself would be a double-free (this exact double-free,
+	// on the 16-byte dataset_t region, was caught by ASan -- bug-1442).
+	rows := ds.rows
+
+	rc := go_push_handler(&decoded, nil)
+
+	// go_push_handler already freed *ds (see above) -- only the payload
+	// xce_job_submit_t it pointed to still needs freeing: free_Dataset()
+	// never touches ->rows, and handleDataset()'s Data_data_jobArray case
+	// (clib.go) never nulls d.rows after use (every element is its own
+	// independent Go-native copy via cJobArrayToModels, mirroring the C
+	// scheduler side's sched_handle_dataset() jobArray case). Freeing it now
+	// is safe since cJobArrayToModels already fully copied every field into
+	// Go-native models.JobInfo before go_push_handler returned.
+	if rows != nil {
+		jobArrayPtr := (*C.xce_job_submit_t)(rows)
+		C.free_JobSubmit_ptr(&jobArrayPtr)
+	}
+
+	return int(rc), nil
+}

src/frontend/apiserver/backend/wire_dispatch_test.go

@@ -0,0 +1,138 @@
+//go:build cgo
+
+package backend
+
+import (
+	"testing"
+
+	"apiserver/store"
+)
+
+// wire-dispatch integration test (job-array-single-wal-record task 5.5):
+// proves the chain
+//
+//	xce_encode()  (real wire bytes, same as apiserver_push_jobarray() sends)
+//	  -> xce_decode()
+//	  -> extract_data()   (task 5.3b's fix)
+//	  -> go_push_handler()  (the REAL top-level CGO entry point xce_backend_
+//	                          handler calls from C -- clib.go -- never
+//	                          exercised by any prior apiserver test, which
+//	                          all call dispatchJobArray()/handleDataset()
+//	                          directly with hand-built Go/C structs)
+//	  -> handlePush() -> handleDataset() -> cJobArrayToModels() (first-ever
+//	     coverage at the CGO-struct level -- see task 5.4's Implementation
+//	     Notes: "cJobArrayToModels() has zero direct unit coverage at the
+//	     CGO-struct level")
+//	  -> dispatchJobArray() (already well covered by TestDispatchJobArray_*
+//	     in handler_test.go -- exercised here for the first time via the
+//	     REAL CGO/wire path rather than a hand-built []*models.JobInfo)
+//
+// delivers exactly N correctly-addressed records and fires task 5.4's
+// confirmation log line exactly once; and that an identical byte-for-byte
+// SECOND delivery (independently re-encoded/re-decoded, the wire-level
+// equivalent of "the same push arrives twice", e.g. after an apiserver
+// reconnect) creates zero new records and logs zero additional lines.
+//
+// The actual cgo plumbing (encode/decode/extract_data/go_push_handler
+// calls) lives in wire_dispatch_support.go, NOT in this file -- the Go
+// toolchain hard-rejects `import "C"` in any *_test.go file (golang/go#13938,
+// confirmed against this repo's go toolchain: cmd/go/internal/modindex/
+// read.go's `if isTest { badGoFile(...) }` check), so this file only calls
+// the plain-Go-signature wrappers wireDispatchBuildPush/wireDispatchDeliver.
+//
+// vtask_entry() (xcectx.c, C side) is `static` and not exported for
+// testing; wireDispatchDeliver mirrors its few lines of push-handling glue
+// verbatim -- every OTHER step (encode, decode, extract_data,
+// go_push_handler and everything it calls) is the real, unmodified
+// production function.
+
+func TestWireDispatch_OnePushProducesNRecordsAndLogsOnce(t *testing.T) {
+	resetStores()
+	prevHandler := gHandler
+	gHandler = &DefaultHandler{}
+	defer func() { gHandler = prevHandler }()
+	logs := captureLog(t)
+
+	pkt, err := wireDispatchBuildPush(52000, 5)
+	if err != nil {
+		t.Fatalf("wireDispatchBuildPush failed: %v", err)
+	}
+	rc, err := wireDispatchDeliver(pkt)
+	if err != nil {
+		t.Fatalf("wireDispatchDeliver failed: %v", err)
+	}
+	if rc != 0 {
+		t.Fatalf("go_push_handler rc = %d, want 0", rc)
+	}
+
+	for i := uint64(0); i < 5; i++ {
+		jobid := uint64(52000) + i
+		got, ok := store.Jobs.Get(jobid)
+		if !ok {
+			t.Errorf("jobid %d should exist in store.Jobs after one real-wire array push", jobid)
+			continue
+		}
+		if got.QueueName != "normal" {
+			t.Errorf("jobid %d: QueueName = %q, want normal", jobid, got.QueueName)
+		}
+	}
+
+	if got := logs.Len(); got != 1 {
+		t.Errorf("expected exactly 1 confirmation log line, got %d", got)
+	}
+}
+
+func TestWireDispatch_RedeliveryCreatesNoNewRecordsAndLogsNothing(t *testing.T) {
+	resetStores()
+	prevHandler := gHandler
+	gHandler = &DefaultHandler{}
+	defer func() { gHandler = prevHandler }()
+	logs := captureLog(t)
+
+	// Two independent build+encode calls with identical inputs, so each
+	// delivery genuinely goes through its own encode and decode -- not one
+	// decode fed to the handler twice.
+	pkt1, err := wireDispatchBuildPush(53000, 3)
+	if err != nil {
+		t.Fatalf("wireDispatchBuildPush (1st) failed: %v", err)
+	}
+	pkt2, err := wireDispatchBuildPush(53000, 3)
+	if err != nil {
+		t.Fatalf("wireDispatchBuildPush (2nd) failed: %v", err)
+	}
+
+	rc, err := wireDispatchDeliver(pkt1)
+	if err != nil {
+		t.Fatalf("first delivery failed: %v", err)
+	}
+	if rc != 0 {
+		t.Fatalf("first delivery rc = %d, want 0", rc)
+	}
+	for i := uint64(0); i < 3; i++ {
+		if _, ok := store.Jobs.Get(53000 + i); !ok {
+			t.Fatalf("jobid %d should exist after first delivery", 53000+i)
+		}
+	}
+	if got := logs.Len(); got != 1 {
+		t.Fatalf("expected exactly 1 confirmation log line after first delivery, got %d", got)
+	}
+	logs.TakeAll() // discard the first delivery's own confirmation line
+
+	// Second, byte-identical delivery (simulated redelivery/reconnect):
+	// every index (53000..53002) already exists in store.Jobs -- must be a
+	// pure no-op, and must NOT log a second confirmation line.
+	rc, err = wireDispatchDeliver(pkt2)
+	if err != nil {
+		t.Fatalf("second (redelivered) delivery failed: %v", err)
+	}
+	if rc != 0 {
+		t.Fatalf("second (redelivered) delivery rc = %d, want 0", rc)
+	}
+
+	if n := store.Jobs.Count(); n != 3 {
+		t.Errorf("store.Jobs count after redelivery = %d, want 3 (no new entries)", n)
+	}
+	if got := logs.Len(); got != 0 {
+		t.Errorf("expected zero confirmation log lines on redelivery, got %d", got)
+	}
+}