From 5331832565a34424773f42acf4d5a969070bfe85 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 19 Jun 2026 15:54:34 +0000 Subject: [PATCH] feat(xce-test-e2e-primitives): auto-var substitution $?/$jobid/$jobcount (task 3.1) --- .kiro/specs/xce-test-e2e-primitives/tasks.md | 4 +- src/controller/testsuite/test.c | 136 ++++++++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/xce-test-e2e-primitives/tasks.md b/.kiro/specs/xce-test-e2e-primitives/tasks.md index bff2d956..45ea56c4 100644 --- a/.kiro/specs/xce-test-e2e-primitives/tasks.md +++ b/.kiro/specs/xce-test-e2e-primitives/tasks.md @@ -25,7 +25,7 @@ - _Depends: 1.1_ - [ ] 3. Auto-variable substitution -- [ ] 3.1 Substitute the fixed auto-vars `$?`, `$jobid`, `$jobcount` +- [x] 3.1 Substitute the fixed auto-vars `$?`, `$jobid`, `$jobcount` - Add `substitute_autovars(line)` (run before tokenizing in `preprocess_and_run`): replace whole-token `$?`→`g_last_status`, `$jobid`→`g_last_jobid`, `$jobcount`→`g_last_jobcount`. A reference to an unset `$jobid`/`$jobcount` fails the command (no garbage substitution). No other variable syntax is recognized. - Observable: after a command, `echo $?` prints the prior status; referencing `$jobid` before any successful `jobsub` fails that command rather than substituting an empty/garbage value. - _Requirements: 3.1, 3.3, 3.5, 3.6_ @@ -88,3 +88,5 @@ - Build dir is `src/xce.build` (not `src/build`). Fast incremental build: `cd src && nix develop --command bash -c "meson compile -C xce.build xce_test"`. Binary: `src/xce.build/controller/xce_test`. Behavioral tests = piped scripts gating on `$?` with `ASAN_OPTIONS=detect_leaks=0`; no controller needed (an unknown command is a convenient failing command). `xce_test` is NOT a meson-registered `test()`, so `meson test --suite controller` does not exercise it. - Pre-existing unrelated failure: `meson test --suite controller` reports `test_demotion_replay_yields_committed_only` FAIL (in `controller_unit_test` / `test_commit_barrier_reply.c`) — from the in-flight `ha-commit-durability` feature, in a binary that does NOT compile `test.c`. Not caused by this spec; ignore it for verdicts here. - 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. diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 7bfc5839..ccc5b951 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -16,6 +16,7 @@ #include #include "client.h" +#include "config.h" /* XCE_MAXNAME_LEN */ #include "controller.capnp.h" #include "delay.h" #include "idgen.h" @@ -133,6 +134,19 @@ static int g_session_failed = 0; static int g_continue_on_error = 0; static int g_last_status = 0; +/* Auto-variable state for the fixed `$jobid` / `$jobcount` substitution + * (xce-test-e2e-primitives task 3.1, Req 3.1/3.3/3.5/3.6). + * + * g_last_jobid most recently acknowledged JobId as a string; empty ("") + * means unset. Populated later by cmd_jobsub (task 4.1). + * g_last_jobcount most recent job count; -1 means unset. Populated later by + * cmd_joblist (task 5.1). + * + * Until those tasks land, both are always unset — which is exactly what makes + * an unset `$jobid`/`$jobcount` reference fail the command (Req 3.6). */ +static char g_last_jobid[XCE_MAXNAME_LEN] = { 0 }; +static int g_last_jobcount = -1; + static idgen_ctx_t idctx; delaymgr_t delaymgr; @@ -405,6 +419,105 @@ int execute_line(char* line) return rc; } +/* Substitute the fixed auto-variables `$?`, `$jobid`, `$jobcount` on a raw line + * before tokenizing (xce-test-e2e-primitives task 3.1, Req 3.1/3.3/3.5/3.6). + * + * Only WHOLE-TOKEN occurrences are recognized: a `$name` must be bounded by the + * start of the line / whitespace on its left and whitespace / end-of-line on its + * right. A `$name` glued to other non-whitespace (e.g. `x$jobid`, `$jobidx`) is + * NOT a recognized auto-variable and is copied through verbatim. No other `$...` + * syntax is recognized: an unknown whole token such as `$other` is also copied + * verbatim (it is simply not one of the three names). + * + * Replacements: + * $? -> decimal of g_last_status (always available). + * $jobid -> g_last_jobid if set (non-empty); unset => command failure. + * $jobcount -> decimal of g_last_jobcount if set (>=0); unset => failure. + * + * Writes the expanded line into `out` (capacity `outsz`, always NUL-terminated + * on success). Returns 0 on success, -EINVAL on an unset auto-var reference, + * -ENOSPC if the expansion would overflow `out`. On any error `out` content is + * unspecified and the caller must fail the command rather than run it. */ +static int substitute_autovars(const char* line, char* out, size_t outsz) +{ + const char* p = line; + size_t o = 0; + + if ((line == NULL) || (out == NULL) || (outsz == 0)) { + return -EINVAL; + } + + while (*p != '\0') { + /* A `$` starts a candidate auto-var only when it begins a token, i.e. + * the previous char is the line start or whitespace. */ + int at_token_start + = (p == line) || (*(p - 1) == ' ') || (*(p - 1) == '\t'); + + if ((*p == '$') && at_token_start) { + const char* name = p + 1; + const char* end = name; + const char* repl = NULL; + char numbuf[32]; + + /* Scan to the token end (whitespace or NUL). */ + while ((*end != '\0') && (*end != ' ') && (*end != '\t')) { + end++; + } + + size_t namelen = (size_t)(end - name); + + if ((namelen == 1) && (name[0] == '?')) { + snprintf(numbuf, sizeof(numbuf), "%d", g_last_status); + repl = numbuf; + } else if ((namelen == 5) && (strncmp(name, "jobid", 5) == 0)) { + if (g_last_jobid[0] == '\0') { + fprintf(stderr, + "autovar: $jobid referenced before set\n"); + return -EINVAL; + } + repl = g_last_jobid; + } else if ((namelen == 8) && (strncmp(name, "jobcount", 8) == 0)) { + if (g_last_jobcount < 0) { + fprintf(stderr, + "autovar: $jobcount referenced before set\n"); + return -EINVAL; + } + snprintf(numbuf, sizeof(numbuf), "%d", g_last_jobcount); + repl = numbuf; + } + + if (repl != NULL) { + size_t rlen = strlen(repl); + + if ((o + rlen) >= outsz) { + return -ENOSPC; + } + memcpy(out + o, repl, rlen); + o += rlen; + p = end; /* consume the whole `$name` token */ + continue; + } + + /* Not a recognized auto-var: copy the whole token verbatim. */ + while (p < end) { + if ((o + 1) >= outsz) { + return -ENOSPC; + } + out[o++] = *p++; + } + continue; + } + + if ((o + 1) >= outsz) { + return -ENOSPC; + } + out[o++] = *p++; + } + + out[o] = '\0'; + return 0; +} + /* Main-loop wrapper around execute_line that implements the session verdict * (xce-test-e2e-primitives task 1.1): * - blank / whitespace-only lines and `#`-comment lines are ignored; @@ -468,7 +581,28 @@ int preprocess_and_run(char* line) return 0; } - rc = execute_line(line); + /* Substitute the fixed auto-variables (`$?`, `$jobid`, `$jobcount`) before + * tokenizing (xce-test-e2e-primitives task 3.1). An unset `$jobid` / + * `$jobcount` reference (Req 3.6) — or an overflow — fails the command: + * we do NOT run execute_line, and the result is treated exactly like a + * failing command (g_last_status non-zero, g_session_failed, abort vs + * continue). The leading `!` shell-escape above is intentionally NOT + * substituted: that line goes raw to system() and the shell owns `$?`. */ + { + char expanded[MAX_LINE_LEN]; + int subrc = substitute_autovars(p, expanded, sizeof(expanded)); + + if (subrc < 0) { + g_last_status = subrc; + g_session_failed = 1; + if (!g_continue_on_error) { + return 1; /* abort: stop the read loop */ + } + return 0; + } + + rc = execute_line(expanded); + } g_last_status = (rc < 0) ? (rc) : 0;