fix(xce-test-e2e-primitives): jobsub clean-ack gating + $jobid capture, closes bug-786 (task 4.1)

This commit is contained in:
Rongsong Shen 2026-06-19 16:04:13 +00:00
parent 5331832565
commit fae0bc0ff4
2 changed files with 58 additions and 7 deletions

View file

@ -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: <id>` 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).

View file

@ -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;