From fae0bc0ff4c78754a195b1ce1207ea0858826dc2 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 19 Jun 2026 16:04:13 +0000 Subject: [PATCH] fix(xce-test-e2e-primitives): jobsub clean-ack gating + $jobid capture, closes bug-786 (task 4.1) --- .kiro/specs/xce-test-e2e-primitives/tasks.md | 3 +- src/controller/testsuite/test.c | 62 ++++++++++++++++++-- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/.kiro/specs/xce-test-e2e-primitives/tasks.md b/.kiro/specs/xce-test-e2e-primitives/tasks.md index 45ea56c4..d605fa17 100644 --- a/.kiro/specs/xce-test-e2e-primitives/tasks.md +++ b/.kiro/specs/xce-test-e2e-primitives/tasks.md @@ -33,7 +33,7 @@ - _Depends: 1.1_ - [ ] 4. Correct job-submission reporting (bug-786) + `$jobid` capture -- [ ] 4.1 `cmd_jobsub` reports success only on a clean ack with a valid JobId +- [x] 4.1 `cmd_jobsub` reports success only on a clean ack with a valid JobId - Gate the `jobsubmit ok: JobId: ` report on a successful reply status AND a well-formed JobId; on success set `g_last_jobid` (feeds `$jobid`, 3.2). Otherwise print a typed error (redirect / no-master / not-ready / timeout) and return `<0`. Never print "ok" with an uninitialized/garbage id (`63-15--1-...`). - Observable: a submit that does not get a clean ack returns failure and does NOT set `$jobid`; `jobsub` never prints success with a malformed id; a successful submit sets `$jobid` to the acked JobId. - _Requirements: 7.1, 7.2, 7.3, 3.2_ @@ -90,3 +90,4 @@ - 1.1: `preprocess_and_run()` is the single chokepoint where the leading-`!` shell-escape (2.1) and `$`-autovar substitution (3.1) plug in. File-scope `g_session_failed`/`g_continue_on_error`/`g_last_status` added; `main` returns `g_session_failed ? 1 : 0`; `set continue-on-error` is a process-global argc==2 special-case in `cmd_set` (not a per-context `settings[]` entry). - Feature Flag Protocol: the flag must be fully REMOVED at the end (no pinned `#define X (1)` guarding a dead `#else`). Reviewer rejects leftover flag scaffolding as dead code. (3.1 hit this — re-mediated.) - 3.1: introduced `g_last_jobid[XCE_MAXNAME_LEN]={0}` (empty = unset) and `g_last_jobcount=-1` (-1 = unset) file-scope state, POPULATED later by 4.1 (jobsub) / 5.1 (joblist). `substitute_autovars(line,out,sz)` does whole-token replace of `$?`/`$jobid`/`$jobcount`; unset `$jobid`/`$jobcount` → command fails (`autovar: $X referenced before set`), no garbage. `!` shell-escape lines are NOT substituted (raw to shell — the shell owns its `$`). Added `#include "config.h"` for XCE_MAXNAME_LEN. +- 4.1 (bug-786 fixed): `cmd_jobsub` inits local `jobid=0`, prints `jobsubmit ok` + captures `g_last_jobid` ONLY after BOTH gates: `rc>=0` AND `jobid!=0`; else typed error (`E_XCE_NOMASTER/REDIRECT/NOT_READY`/timeout switch via `xce_strerror_r`) + `return <0`. **`$jobid` capture uses `fmt_jobid(g_last_jobid, ..., jobid)` — the FULL jobid, byte-identical to how `cmd_joblist` formats `jobs[i]->jobid`** (NOT `array_jobid()`, which the display-only `ok:` line uses). This is REQUIRED so task 5.3 `expect-job $jobid` matches the joblist column for array jobs. Also moved the error check AFTER the FREEUP block → fixed a pre-existing error-path leak. Live success-path ($jobid==joblist entry) verified by code reading; runtime-verified via soak (7.2). diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index ccc5b951..51a1dd26 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -1144,7 +1144,11 @@ int cmd_jobsub(int argc, char* argv[]) job_submit_data_t submit; xce_job_submit_t jobsubmit = { .submit = &submit }; int opt; - uint64_t jobid; + /* Initialize so a "rc >= 0 but *jobid never written" outcome (a non-clean + * ack such as a redirect/zero-row/not-ready surfaced as a non-negative rc) + * is detectable as an invalid (zero) JobId rather than printed as garbage + * (bug-786). */ + uint64_t jobid = 0; uint32_t min = 1, max = 1; uint32_t arsize = 1; char jobidStr[128]; @@ -1254,11 +1258,6 @@ int cmd_jobsub(int argc, char* argv[]) jobsubmit.array_size = arsize; rc = xcecli_jobsubmit(active_ctx->ctx, &jobsubmit, &jobid); - if (rc < 0) { - SET_ERROR(-rc); - perror("xcecli_jobsubmit"); - return rc; - } FREEUP(submit.user); FREEUP(submit.queue); @@ -1267,8 +1266,59 @@ int cmd_jobsub(int argc, char* argv[]) FREEUP(submit.start_info->exdata); FREEUP(submit.start_info); + /* Report success only on a clean ack with a well-formed JobId (bug-786, + * Req 7.1/7.2/7.3). xce_jobsubmit() writes *jobid only when the reply + * status is >= 0; XCECLI_FAILOVER_CALL surfaces a non-clean outcome as a + * negative rc (e.g. -E_XCE_NOT_READY on no-primary budget exhaustion, + * -E_XCE_REDIRECT / -E_XCE_NOMASTER on a standby, -ETIMEDOUT / -ENOTCONN on + * transport loss). A clean ack is therefore rc >= 0 AND a non-zero jobid; + * anything else is a typed failure and must NOT print "ok" or set $jobid. */ + if (rc < 0) { + char ebuf[128]; + + ebuf[0] = '\0'; + xce_strerror_r(-rc, ebuf, sizeof(ebuf)); + SET_ERROR(-rc); + + switch (-rc) { + case E_XCE_NOMASTER: + fprintf(stderr, "jobsubmit failed: no-master (%s)\n", ebuf); + break; + case E_XCE_REDIRECT: + fprintf(stderr, "jobsubmit failed: redirect (%s)\n", ebuf); + break; + case E_XCE_NOT_READY: + fprintf(stderr, "jobsubmit failed: not-ready (%s)\n", ebuf); + break; + case ETIMEDOUT: + fprintf(stderr, "jobsubmit failed: timeout (%s)\n", ebuf); + break; + default: + fprintf(stderr, "jobsubmit failed: %s\n", ebuf); + break; + } + + return rc; + } + + if (jobid == 0) { + /* Non-negative rc but no JobId was written: not a clean ack. Never + * print "ok" with an uninitialized/garbage id (Req 7.3). */ + fprintf(stderr, + "jobsubmit failed: no clean ack (no JobId returned)\n"); + return -E_XCE_INTERNAL; + } + fmt_jobid(jobidStr, sizeof(jobidStr) - 1, array_jobid(jobid)); + /* Capture $jobid (Req 3.2) using the SAME representation cmd_joblist prints + * for a job's JobId column — fmt_jobid(jobs[i]->jobid), the full jobid + * WITHOUT array_jobid() stripping — so a later `$jobid` substitution and + * `expect-job $jobid` compare equal to the joblist entry for this job. Only + * set on this clean-ack + valid-id path; never on a failure path, so an + * unset $jobid still fails per Req 3.6. */ + fmt_jobid(g_last_jobid, sizeof(g_last_jobid) - 1, jobid); + printf("jobsubmit ok: JobId: %s\n", jobidStr); return 0;