docs(xce-test-cli-ux): verify the confirmation gate end-to-end (task 4.2)

Live-cluster validation, no source changes: independently reproduced
(implementer + reviewer, separate deploys) that all five destructive
commands prompt by default, treat decline and real-SIGINT identically as
"not run" (not "failed", the deliberate Req 9.9 exception to Req 1.3),
skip the prompt under --yes/non-interactive/session-toggle, and match
Req 9.5's exact enumeration including its reversal-action exclusions.

Logs bug-1796 (misleading perror() text from stale errno in
cmd_qctrl/hostctrl/jobkill) and bug-1797 (cmd_qctrl queue-name leak on
the RPC-error path) as pre-existing, deliberately-deferred findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
Rongsong Shen 2026-09-10 03:37:01 +00:00
parent 18aa736665
commit f6793a24f1
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 53 additions and 1 deletions

View file

@ -157,12 +157,13 @@
- _Depends: 3.2, 1.7_
- _Done (requirements 1.1-1.5 MET): run twice independently (implementer + reviewer), each against a separately-deployed single-node live controller (`/xce-deploy-ha`-style, `deploy-ha.sh`), freezing the controller mid-RPC with `SIGSTOP` to create a genuine blocked-request condition and sending real `SIGINT` to the real `xce_test` process. AC 1.1: a `hello` blocked against the frozen controller (confirmed hung, no reply) was abandoned in ~0.00026-0.00028s of wall-clock time after SIGINT (both runs closely agreed), vs. the ~10s `cbuf_deadline_10s` RPC timeout. AC 1.2: a partial, un-submitted input line was discarded cleanly on SIGINT, prompt redisplayed, session immediately usable. AC 1.3: the interrupted command reported `hello: Interrupted system call` on stderr and `$?` == -4 (-EINTR), the same failure signaling as any other failing command. AC 1.4: the process survived 3 separate SIGINTs across the run, exiting only on an explicit `quit`. AC 1.5: a second `hello` issued immediately after the first abandonment, on the same connection, with the controller still frozen, itself blocked (queuing on the per-connection lock) rather than racing or returning stale data; after `SIGCONT`, a third `hello` on that same connection completed normally, proving full reuse after two abandonments. The different-connection-independence sub-clause was validated via existing white-box coverage (`test_different_connection_keys_run_independently`, `test_interruptcall.c`) rather than a second live controller node — both implementer and reviewer independently read that test and confirmed it exercises the same connection-key abstraction production uses (`active_ctx->ctx`) with a real abandonment on one key and asserted concurrency on a distinct key, and design.md confirms the mechanism is entirely client-side, so a second live node would add no further assurance. No source files were changed; both deploys were fully cleaned up (processes killed, deploy dirs removed, confirmed via `pgrep`)._
- [ ] 4.2 Verify the confirmation gate end-to-end
- [x] 4.2 Verify the confirmation gate end-to-end
- Confirm each of the five destructive commands prompts by default, respects its bypass flag and the session toggle, and auto-skips under non-interactive input
- Confirm that a real interrupt signal delivered while the confirmation prompt is displayed is observed as an interruption and reported as not run, not as failed
- Observable completion: a live end-to-end run against each destructive command demonstrates the prompt, the bypass, the toggle, the non-interactive auto-skip, and the real-signal-during-prompt case
- _Requirements: 9.1, 9.2, 9.3, 9.4, 9.5, 9.6, 9.7, 9.8, 9.9_
- _Depends: 3.3, 1.7_
- _Done (requirements 9.1-9.9 MET): run twice independently (implementer + reviewer), each against separately-deployed live single-node clusters (one with a real worker), for all five destructive commands (jobkill, shutdown, qctrl -i/-c, hostctrl -c, upgradeconfig). Default prompt + decline reports "<cmd>: not run (declined)" on stderr only, never "failed" (AC1/AC2/AC8). A real SIGINT delivered while blocked inside the prompt's read produces the IDENTICAL "not run (declined)" text for all five (AC9) -- concretely contrasted against task 4.1's own finding that Ctrl-C on an ordinary (non-destructive) command instead reports "<cmd>: Interrupted system call" as a failure, proving AC9's exception to Requirement 1.3 is real, not assumed. `--yes` and non-interactive (piped, non-tty) input both skip the prompt and proceed straight to the RPC for all five (AC3/AC4), including full real round trips for jobkill (a real submitted job actually signaled) and upgradeconfig (the real generated cluster.bin actually resubmitted, 3 times, safely idempotent) and, via two disposable throwaway controllers, shutdown itself (actually terminated for real to prove the bypass path executes all the way, not just "reached the RPC call"). The session toggle `set -y`/`set +y` (AC6/AC7) works for all five once issued from the top-level context (`set` is registered only in `top_commands[]`, not `client_commands[]`/`worker_commands[]`/`scheduler_commands[]` -- a cosmetic UX gap, not a Requirement 9 defect, confirmed by both runs). AC5's exact enumeration verified live both ways: `qctrl -a`/`-o`, `hostctrl -o`, and worker-context `jstart` never prompt; `qctrl -i`/`-c` and `hostctrl -c` do. No defect found in the confirmation gate itself. Two genuine but pre-existing, out-of-boundary defects were found along the way and logged rather than fixed: bug-1796 (cmd_qctrl/cmd_hostctrl/cmd_jobkill's RPC-error perror() calls print stale/misleading errno text, root-caused and reproduced independently by the reviewer, confirmed unrelated to the interrupt adapter or confirmation gate) and bug-1797 (cmd_qctrl leaks its strdup'd queue name on the RPC-error return path, ASAN-confirmed). No source files were changed; both sets of deploys were fully cleaned up._
- [ ] 4.3 Verify source's interaction with the confirmation gate
- Confirm a destructive command inside a sourced file, run within an otherwise-interactive session, still prompts for confirmation unless the session toggle is set, proving the gate's interactive check is based on the top-level session rather than on whether the current line came from a file

View file

@ -21591,6 +21591,57 @@
],
"occurrences": 1,
"last_seen": "2026-09-09T20:10:00Z"
},
{
"id": "bug-1796",
"timestamp": "2026-09-09T22:00:00Z",
"error_message": "cmd_qctrl/cmd_hostctrl/cmd_jobkill's RPC-failure branches perror() a fixed label (e.g. \"xcecli_queuectrl\") without ever setting errno to reflect the RPC's actual returned status, so the printed reason (e.g. \"Interrupted system call\", \"Resource temporarily unavailable\", \"Inappropriate ioctl for device\") is whatever errno happened to be left over from an unrelated earlier libc call in the same process, not the real failure reason.",
"file": "src/controller/testsuite/test.c (cmd_qctrl, cmd_hostctrl, cmd_jobkill RPC-error return paths)",
"root_cause": "Each of these commands does `if (rc < 0) { perror(\"xcecli_...\"); return rc; }` where `rc` is the RPC's own negative-errno-style return value (e.g. a clean -ENOENT for \"no such queue/host\"), but perror() always reads the CURRENT process errno, not `rc` -- and nothing in this branch ever does `errno = -rc` first. Found during xce-test-cli-ux task 4.2's live confirmation-gate verification: a bypassed qctrl/hostctrl RPC against a deliberately bogus (nonexistent) queue/host name printed different, unrelated-looking error text across different runs of the SAME scenario, root-caused (and independently reproduced 3 times by the task 4.2 reviewer) to stale errno left over from prior, unrelated syscalls/signal-decline paths in the same process -- e.g. a destructive-command Ctrl-C decline explicitly sets errno = EINTR at its own call site and never resets it, so a LATER unrelated RPC failure in the same session then misreports itself as \"Interrupted system call\". Confirmed pre-existing (commit 41f56fa6e, 2025-11-05), predating the whole xce-test-cli-ux spec; NOT a defect in the interruptible-call adapter (tasks 1.4/3.2) or the confirmation gate (task 2.1/3.3) -- both were independently confirmed to behave correctly in every case tested, this is purely a misleading-diagnostic-text bug.",
"fix": "NOT YET FIXED (found and deliberately deferred -- out of boundary for task 4.2, a pure live-verification task with no source changes permitted). Recommended: set `errno = -rc;` immediately before each `perror(...)` call on these RPC-error paths (or replace with `fprintf(stderr, \"...: %s\\n\", strerror(-rc));` to avoid the errno indirection entirely), and audit other cmd_* handlers for the same perror()-without-errno-set pattern.",
"tags": [
"controller",
"testsuite",
"xce_test",
"cmd_qctrl",
"cmd_hostctrl",
"cmd_jobkill",
"perror",
"errno",
"misleading-diagnostic",
"xce-test-cli-ux",
"task-4.2",
"pre-existing",
"deferred"
],
"related_bugs": [],
"occurrences": 1,
"last_seen": "2026-09-09T22:00:00Z"
},
{
"id": "bug-1797",
"timestamp": "2026-09-09T22:00:00Z",
"error_message": "cmd_qctrl leaks its strdup()'d queue name on the xcecli_queuectrl() RPC-error return path (ASAN LeakSanitizer-confirmed, 14 bytes).",
"file": "src/controller/testsuite/test.c (cmd_qctrl)",
"root_cause": "cmd_qctrl does `queue = strdup(optarg)` while parsing -q, then on `if (rc < 0) { perror(\"xcecli_queuectrl\"); return rc; }` (the RPC-error branch) returns without `free(queue)` -- only the success path frees it. Distinct from the adjacent, intentional non-free on the EINTR/abandoned-worker path (ownership legitimately transferred there per the interruptible-call adapter's contract). Found during xce-test-cli-ux task 4.2's live confirmation-gate verification (a bypassed qctrl against a bogus queue name triggered this exact RPC-error branch under the asan build). Confirmed pre-existing (commit 41f56fa6e, 2025-11-05), predating the whole xce-test-cli-ux spec and untouched by task 3.1's flagspec migration; not caused by any change in this spec.",
"fix": "NOT YET FIXED (found and deliberately deferred -- out of boundary for task 4.2, a pure live-verification task with no source changes permitted). Recommended: add `free(queue);` on the RPC-error return path (or restructure with a single cleanup label), and check `cmd_hostctrl`/`cmd_jobkill`'s sibling strdup'd-argument fields for the same pattern.",
"tags": [
"controller",
"testsuite",
"xce_test",
"cmd_qctrl",
"leak",
"asan",
"xce-test-cli-ux",
"task-4.2",
"pre-existing",
"deferred"
],
"related_bugs": [
"bug-1796"
],
"occurrences": 1,
"last_seen": "2026-09-09T22:00:00Z"
}
]
}