fix(scheduler): clock/resval/jobid-format fixes plus REPL live-operator gaps (task 12.5)

Bugs fixed, discovered across a day of live-operator testing against a
real deployed cluster:

- bug-1584: clock anchors never initialized to real wall-clock values,
  timestamps showed system uptime instead of Unix epoch.
- bug-1585: heap-use-after-free in the normal-mode driver's periodic
  resval update path -- core_events.c's RESVALS_FULL/_DELTA handlers do a
  shallow zarray_put() of the wire-decoded xce_val_t*, but driver.c's
  release_translated_payload() unconditionally freed the same decoded
  array right after, so the next periodic report for the same
  worker+resource triggered a UAF read via kill_sched_resval(). New
  regression test: test_resval_ownership.c.
- bug-1587/1588/1591: raw-ns/raw-uint64 jobid/timestamp prints across the
  REPL/SIM family converted to the same human-readable formatting already
  used elsewhere (fmt_jobid(), timestamp formatting).
- bug-1590: repl_info.c's XCE_VAL_NUMERIC checks corrected to
  Value_u_fvalue, fixing `info worker`/`info resource` falling back to
  0.00 for real numeric resvals.

task 12.5 (Req 5.22 amendment) -- five REPL gaps surfaced by a live
operator session, each with its own bug id:
- bug-1594: `continue`/`step` reaching EOF with no breakpoint hit now
  re-enters the REPL via xce_repl_on_replay_complete() instead of exiting
  the process; step/continue issued after replay is already complete
  re-prompt instead of silently exiting.
- bug-1595: `step`/`continue` now echo each input event (not just
  decisions) via repl_print_event_summary(), in real event/decision
  order.
- bug-1593: `info resource <name>|all` now reports resource definition
  metadata restricted to XCE_RESF_GLOBAL/XCE_RESF_SHARED resources, read
  from core->sharedInst, instead of aggregating host-based resources
  across every worker.
- bug-1596: repl_split_tokens() strips one matching pair of surrounding
  quote characters from each token.

See requirements.md's Req 5 AC 22 "Amendment (2026-08-19)" for the full
rationale on each of the five task 12.5 gaps.

Full scheduler suite green (41/42; the one remaining failure,
test_recordstart_symlink_escape_replies_error, is the pre-existing
unrelated bug-1560).
This commit is contained in:
Rongsong Shen 2026-08-19 09:48:46 +00:00
parent 515f4ca098
commit c9acd502c0
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
32 changed files with 2547 additions and 295 deletions

View file

@ -1106,6 +1106,18 @@ if (arguments.sim_file) {
**Implementation note**: The REPL is a single `fgets` loop in the simulation driver. It is not a separate thread — when the REPL blocks on stdin, the simulation loop is parked. This is correct and intentional: the operator is in control, and the simulation has no wall-clock deadlines.
**Amendment (2026-08-19, live-operator testing — task 12.5)**: driving the REPL against a real replay file surfaced five gaps in the v1 design above. All five are now part of this acceptance criterion, not a follow-up:
1. **`continue` with no breakpoints must not end the session.** The original pseudocode's `for each event in replay_file: ...` loop, once it reaches EOF with no breakpoint ever hit, simply falls out the bottom and the process exits — the operator never gets to inspect the final state. Corrected behavior: reaching EOF while `--interactive` prints the existing "[sim] replay complete. N_sessions processed." banner and then **re-enters the REPL command loop** instead of returning. State-inspection commands (`info job/queue/worker/resource/summary/session`) remain fully usable against the final simulated state.
2. **The simulator shall never terminate except via an explicit `quit` (or stdin EOF, which this REPL has always treated as an implicit `quit`).** This is the direct corollary of (1): once replay is complete and the REPL has re-entered its prompt, `step`/`continue` at that point are no-ops that print "[sim] replay already complete -- no more events to process. Use 'quit' to exit." and re-prompt, rather than being treated as "hand control back to a driver loop that no longer has anything to drive."
3. **`step`/`continue` shall echo the input events and resulting decisions for the current session, in the order they actually occur.** V1 only ever printed decisions (`sim_print_decision()`); the events that drove them (`JOB_PUSH`, `JOB_STATE_CHANGE`, `WORKERS_PUSH`, etc.) were completely silent unless a breakpoint happened to match one. An operator single-stepping through a session had no visibility into what changed before the resulting dispatch/pend decisions appeared. Corrected behavior: every event is echoed one line at a time as it is decoded (the same point breakpoint-matching already inspects it, `xce_repl_on_event()`), so the printed sequence is event(s) → BEGIN_ROUND → decision(s), matching real execution order.
4. **`info resource <name>|all` shall report resource *metadata*, restricted to globally-shared resources — never a per-host aggregation.** V1's implementation summed `total`/`available` across every worker's per-host resource value for every resource, host-based ones included (e.g. `cpus`, `mem`) — a fabricated, meaningless number for a per-host consumable already correctly inspected via `info worker <name>`, and it never surfaced the resource *definition* (description, value type, which of `XCE_RESF_BUILTIN`/`DYNAMIC`/`GLOBAL`/`SHARED`/`EXTERNAL`/`RELEASE`/`DEFAULT_HOST`/`CONSUMABLE`/`LIMIT` apply, reporting interval). Corrected behavior: `info resource <name>` prints the resource's definition metadata; `info resource all` lists only resources flagged `XCE_RESF_GLOBAL` or `XCE_RESF_SHARED` (host-based/per-worker resources are out of scope for this command — they are `info worker`'s job). A global/shared resource's current value is read from the core's single shared-instance pool (`core->sharedInst`, indexed by the resource's `idx` — the same pool `RESVALS_FULL`/`RESVALS_DELTA` events and snapshot save/load already treat as the defining store for these resources), never aggregated across workers.
5. **REPL command arguments accept an optional pair of surrounding quotes.** An operator copying a jobid straight out of `info jobs`' own output (e.g. `0-15-1787129769-0[0]`) and habitually quoting it (`info job "0-15-1787129769-0[0]"`) got a silent wrong-answer failure (`parse_jobid()`'s regex requires a whole-string anchor match, so the literal quote characters broke the match, and the `strtoull()` fallback parsed the leading `"` as a nonnumeric prefix, silently producing jobid 0) instead of an intelligible error. Corrected behavior: `repl_split_tokens()` strips one matching pair of leading/trailing `"` or `'` characters from each token before it reaches any command handler.
### Requirement 6: Object sharing forward-compatibility
**Objective:** As a maintainer, I want the core's data-ownership patterns to leave room for future resreq/pend-reason object sharing, so that a later optimization spec can introduce sharing without retrofitting these invariants under load-bearing code.

View file

@ -389,6 +389,13 @@
- _Depends: 12.2, 12.3_
- _Verified: 2026-08-07 — TEST-ONLY task, no new production code. New test_repl_equivalence.c (683 lines) runs the SAME sched_sim_driver_run() (sim_loop.c) against ONE hand-built `.replay` fixture (two queues at different priorities "high"=5/"normal"=1, four jobs split across THREE ReplaySession boundaries) twice: once in batch mode (interactive=false, straight through), once REPL-driven (interactive=true, stdin freopen()'d from a scripted "step 1\ncontinue\n" -- exactly 2 lines, sufficient and safe regardless of session count since CONTINUE mode never re-reads stdin once entered, avoiding the readline()-EOF-triggers-exit(0) trap). This is a DIFFERENT comparison than task 11.6's test_sim_equivalence.c (which compares normal-mode driver vs simulation driver) -- here both sides are the identical simulation driver, differing only in `sim->interactive`. Both runs set `sched_sim_args_t.output_path` directly (confirmed unconditional on `interactive` by reading sim.c's `args->output_path != NULL` gate), so both write a full `.replay`-format decision stream to their own file with no `set output` REPL command needed. Since the fixture has THREE session boundaries (not task 11.6's one), decoding uses a new decode_all_sessions() that scans the whole output file structurally via sim.c's shared `msg_root_has_shape()` (ReplaySession's distinct (1,2) word/pointer-count shape) rather than a fixed message-skip count. Comparator reuses sim_verify.c's leaf-level structural comparators (`sim_verify_job_alloc_eq()`/`sim_verify_job_preason_eq()`, both `static` but visible via white-box #include) through a duplicated `replay_decision_content_eq()`, same technique test_sim_equivalence.c's own function of that name establishes. Non-tautology guards: session 1 asserted to have EXACTLY 2 decisions (an unambiguous floor -- only 2 jobs could possibly be pending that early), job 501 independently pinned as present in session 1's batch-side content, total batch decisions asserted >= 4, and every decision's type independently pinned as PEND (matching sim_picker_thread_fn()'s documented DISPATCH gap) BEFORE the cross-run equality check -- so a buggy always-`return 1` comparator could not pass silently. Observed (not hand-predicted) that a PEND'd job persists as a picker candidate into later sessions (session 1: jobs 501/502 -> 2 decisions; session 2: 501/503/502 -> 3; session 3: 501/503/502/504 -> 4; total 9 on both sides), confirming the fixture is genuinely non-trivial without needing a fragile hardcoded per-session prediction. Synthetic RED-phase probe: analysis first showed that defeating `repl_should_pause()` (task 12.1's own already-verified probe target) would NOT turn this test red, because that only changes *when* stdin is read/control returns, never *what* gets processed -- every session still eventually completes regardless of pause timing, and this file's own CONTINUE-is-sticky script design tolerates any pause schedule. The correct probe for THIS invariant ("never alters ... what events are processed") had to touch content, and only under `sim->interactive`: temporarily added one line to `xce_repl_on_event()` (repl_break.c) -- which takes a non-const `xce_core_event_t*` and fires (via repl.h's `sim->interactive`-gated wrapper) strictly before an event joins the batch `xce_core_push_events()` later applies -- corrupting a JOBS_PUSH job's jobid (`+= 1000000`) whenever interactive. Rebuilt, ran the full scheduler suite: exactly 4/37 failed (this new test plus three pre-existing REPL tests that also exercise JOBS_PUSH under interactive mode -- sched_repl_execution_control_test, sched_repl_snapshot_params_test, sched_repl_breakpoints_inspection_test -- all correctly detecting the same real divergence), all 33 others stayed green. This test's own failure output: "REPL step/continue-to-completion must produce a byte-identical decision to batch mode at this session and position" (jobid 501 on the batch side vs 1000501 on the REPL side, directly visible in the two runs' printed PEND lines). Reverted (diff-confirmed byte-identical to the pre-probe file), rebuilt, reconfirmed 37/37 scheduler suite (was 36/36), whole-project build clean.
- [ ] 12.5 REPL live-operator-testing fixes (Requirement 5.22 amendment)
- Fix the five gaps a live operator session surfaced (see requirements.md's AC 22 "Amendment (2026-08-19)" for the full rationale on each): (1) `continue` reaching EOF with no breakpoint hit re-enters the REPL instead of exiting the process; (2) `step`/`continue` issued after replay is already complete re-prompt with a message instead of being treated as "hand control to a driver loop"; (3) `step`/`continue` echo each input event (not just decisions) as it is processed, in real event/decision order; (4) `info resource <name>|all` reports resource definition metadata restricted to `XCE_RESF_GLOBAL`/`XCE_RESF_SHARED` resources, reading the shared value from `core->sharedInst` instead of aggregating host-based resources across every worker; (5) `repl_split_tokens()` strips one matching pair of surrounding quote characters from each token.
- Observable: `--sim --interactive` against a replay file with no breakpoints set, driven by `continue`, stays at the `(sim) ` prompt after "[sim] replay complete" instead of the process exiting -- `info summary`/`info job`/etc. still work at that point, and only `quit` (or stdin EOF) ends the process; a `step` while events are pending prints each event's own one-line summary before that session's decisions; `info resource all` lists only global/shared resources with metadata fields, never a host-based resource's fabricated total/available; `info job "<jobid>"` (quoted) parses identically to the unquoted form.
- _Requirements: 5.22_
- _Depends: 12.4_
- _Bugs: bug-1593 (info resource host-vs-shared), bug-1594 (continue/no-quit exit), bug-1595 (events not echoed in step/continue), bug-1596 (quoted REPL argument parsing)_
- [x] 13. Run full cross-phase HA soak regression
- Run sequential and chaotic HA soak rounds against the complete feature (all three phases integrated), confirming zero job loss, divergence, or liveness stall attributable to this feature across the full run
- Observable: the soak run completes with a clean pass across all rounds, matching the zero-regression bar set by the Phase 1 and Phase 2 soak runs