diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 84985338..84bfa0ed 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -17514,26 +17514,31 @@ }, { "id": "bug-1926", - "status": "open", - "status_updated": "2026-10-08", - "status_note": "newly logged", + "status": "fixed", + "status_updated": "2026-10-09", + "status_note": "", "component": "scheduler", "severity": "wrong-result", "timestamp": "2026-10-08T16:08:34Z", "last_seen": "2026-10-08T16:08:34Z", "occurrences": 1, "error_message": "xce_scheduler --sim REPL break jobid= filter ignores the dashed jobid form the REPL prints\n\n**Observed:** a `break DECISION jobid=` (or `break jobid=`) breakpoint set with the dashed jobid form the REPL itself prints never fires. With the 1.1 contention fixture:\n```\n[sim] breakpoint 1 set: DECISION jobid=0-0-0-104\n-- (continue runs to the end; no hit)\n[sim] breakpoint 1 set: DECISION jobid=104\n[sim] hit breakpoint 1 @ decision: PEND jobid=0-0-0-104\n```\nThe hit line prints `jobid=0-0-0-104`, yet only the raw decimal packed id (`jobid=104`) matches. The breakpoint is accepted without any warning.\n\n**Expected:** a jobid filter accepts the same jobid forms the REPL prints and `info job` accepts (`parse_jobid`/`parse_jobsel`, including `X[k]` array elements), as well as the raw decimal id.\n\n**Impact:** wrong result, usability. An operator copies a jobid from REPL output into `break ... jobid=`, and the breakpoint silently never stops the run. For real jobids (non-zero cluster/user/time fields) the raw decimal form is a large number nobody types, so jobid breakpoints are effectively unusable. No effect on scheduling.\n\n**Found by:** spec task scheduler-sim-decision-trace 6.1 (end-to-end tests had to use raw decimal ids for breakpoints), confirmed by the 6.1 review. It predates that feature: `repl_u64_str_eq` comes from commit 2fecea35 (2026-08-07).\n\nSuspected: `repl_u64_str_eq()` (src/scheduler/src/repl_break.c:210) formats the packed jobid with `%llu` and compares strings, so only the decimal form can match. It is used by the decision filter (`repl_decision_matches_filter`, ~line 346) and, per its comment, by every `jobid=` filter including event filters.", - "file": "src/scheduler/src/repl_break.c", - "root_cause": "TODO: required for fixed/partial", - "fix": "TODO: required for fixed/partial\n\nVerification:\n- Test: TODO: name + file; RED before the fix, GREEN after\n- Suites: TODO: suite results and build profile\n- Live: TODO: soak/deploy run, or n/a and why", + "file": "src/scheduler/src/repl_break.c, src/scheduler/src/repl_fmt.c, src/scheduler/src/repl_internal.h, src/scheduler/src/repl_trace_store.c, src/scheduler/testsuite/test_repl_breakpoints_inspection.c", + "root_cause": "`repl_u64_str_eq()` (src/scheduler/src/repl_break.c) rendered the packed 64-bit jobid with `%llu` and `strcmp()`ed it against the filter's text, so a `jobid=` filter matched only when the operator typed the raw packed decimal id. Every jobid filter went through it: `repl_decision_matches_filter()` (DECISION) and `repl_event_matches_filter()` (JOB_STATE_CHANGE, JOB_UPDATE, JOB_RUSAGE, and each job of a JOB_PUSH). The REPL prints jobids with `fmt_jobid()` (`0-0-0-104`, `X[k]` for array elements, since bug-1591), and `info job`/`info trace`/`trace on job` parse that form, so an id copied from the REPL output never compared equal. `repl_cmd_break()` stored the value as an uninterpreted string and never checked it, so nothing told the operator the breakpoint could not fire.\n\nSince: commit 2fecea35 (scheduler-core task 12.2, 2026-08-07), which added `repl_u64_str_eq`; it became visible once bug-1591 switched the REPL's own output to the dashed form.\n\nScope: checked every caller of `repl_u64_str_eq` (the four event cases and the decision case above; there were no others). `state=`, `type=` and `queue=` filters compare names, not jobids, and are unaffected. `info job` (`parse_jobid`), `info trace` and `trace on job` (`repl_parse_jobsel_arg`, bug-1930) already parsed jobids.", + "fix": "- `repl_parse_jobsel_arg()` moved from src/scheduler/src/repl_trace_store.c to src/scheduler/src/repl_fmt.c (declaration stays in repl_internal.h), next to a new `repl_jobsel_matches(sel, raw, jobid)`. repl_fmt.c holds the REPL's pure shared helpers and is linked into every build that has repl_break.c or the trace store, so breakpoints use the jobid parser without depending on the trace store. Leaving it in repl_trace_store.c would have made the older breakpoint module depend on the trace store.\n- `xce_repl_filter_t` (repl_internal.h) gains `jobsel` and `jobsel_raw`. `repl_cmd_break()` (repl_break.c) parses a `jobid=` value once, when the breakpoint is set. A value the parser rejects prints `[sim] break: invalid jobid '': ` and no breakpoint is created, like a malformed filter. `value` keeps the text as typed, so the \"breakpoint N set\" line and `info break` still echo what the operator wrote.\n- `repl_u64_str_eq()` is replaced by `repl_filter_jobid_matches()`, which calls `repl_jobsel_matches()` on the parsed selector. The event and decision filter functions take the filter struct instead of key/value strings. So every `jobid=` filter, event or decision, uses the same parse and the same matching.\n- Matching semantics (repl_jobsel_matches): a raw packed decimal id matches exactly that id, as before the fix; `X[k]` matches element k of array X; `X[a-b]` matches elements a..b; a bare dashed id matches the plain job with that id and, when the id is array-aligned, any element of the array with that id. That is the set `trace on job` selects (scheduler-sim-decision-trace Requirements 1.2/1.3, tasks.md Implementation Note 1.3). The raw form keeps exact-id matching, not the bare-id array widening, so existing scripts behave exactly as before. The packed id of an element carries the array tag, so a raw element id still names that one element.\n- Rejected alternatives: re-rendering the jobid with `fmt_jobid()` and comparing strings would not cover `X[a-b]` ranges or a bare array id, and would still accept values that can never match. Parsing at match time would re-parse on every event and decision, and could not report a bad value, because the match runs inside the driver loop with no error channel.\n- Behaviour change: a `jobid=` value that is neither a jobid form nor a decimal (e.g. `jobid=bogus`, `jobid=`, a reversed range) is now refused. No committed test or script relied on that: they all use raw decimal ids (`12345`, `42`, `222`, `105`, the 1.1 baseline script's `104`). `help` does not document the filter value format, so it is unchanged.\n\nVerification:\n- Test: test_repl_breakpoints_inspection.c (sched_repl_breakpoints_inspection_test), 7 new tests. RED before the fix, 40 tests 6 failures (`/home/claude/tmp/bug-1926/unit.red.log`):\n ```\n test_decision_breakpoint_jobid_dashed_form_matches:FAIL: jobid=0-0-0-104 must match job 104\n test_decision_breakpoint_jobid_element_and_range_match:FAIL: X[2] must match element 2\n test_decision_breakpoint_jobid_bare_array_id_and_raw:FAIL: a bare id matches the plain job\n test_break_invalid_jobid_rejected:FAIL: Expected 0 Was 1. break DECISION type=PEND jobid=bogus\n test_event_breakpoint_jobid_dashed_form_matches:FAIL: Expected Non-NULL\n test_decision_breakpoint_dashed_jobid_pauses_run:FAIL: (captured run: \"PEND jobid=0-0-0-555\" printed, no \"hit breakpoint 1\")\n ```\n `test_info_break_echoes_jobid_filter_as_typed` passed before and after; it guards the listing. GREEN after the fix: 40 tests 0 failures (`unit.green.log`). The tests cover dashed decision and event (JOB_STATE_CHANGE, JOB_UPDATE, JOB_RUSAGE, JOB_PUSH) filters, `X[k]` copied from `fmt_jobid()`, `X[a-b]`, a bare array id (plain job and elements), raw decimal (exact, including an element's packed id), nine rejected values, and an in-process `sched_sim_driver_run()` with a dashed decision breakpoint.\n- Suites: `bash src/tools/xce-build.sh scheduler --jobs 12 --incremental` (asan, src/xce.build): scheduler suite 118/118 OK, 0 fail (`/home/claude/tmp/bug-1926/gate.log`). It includes the repl trace baseline test: the committed 1.1 transcript is unchanged, since its script uses the raw `jobid=104`. Also the trace e2e, trace cmd, trace store and sim isolation tests.\n- Live: real `xce_scheduler --sim --interactive` binary on the committed contention fixture. Before (`repro.before.out`, the pre-fix binary at logging time): `jobid=0-0-0-104` printed only \"breakpoint 1 set\", and only `jobid=104` hit. After (`repro.after.out`, `sh repro.sh `): both `jobid=0-0-0-104` and `jobid=104` print `[sim] hit breakpoint 1 @ decision: PEND jobid=0-0-0-104`. `repro-forms.sh` (`repro-forms.after.out`): `break JOB_STATE_CHANGE jobid=0-0-0-102` hits at event #6; `break JOB_PUSH jobid=0-0-0-108` hits at event #7; `jobid=0-0-0-104[` and `jobid=bogus` are refused with `[sim] break: invalid jobid '...'`, and `info break` then reports no breakpoints; `info break` lists `jobid=0-0-0-105 type=DISPATCH` and `jobid=104` as typed. All evidence is in `/home/claude/tmp/bug-1926/`.", "tags": [ "sim", "repl", "breakpoint", "jobid" ], - "related_bugs": [], - "fix_commits": [], + "related_bugs": [ + "bug-1930", + "bug-1591" + ], + "fix_commits": [ + "7aef6b85" + ], "record": ".wolf/bugs/bug-1926.md", "gitea_issue": 351, "gitea_synced_status": "open" diff --git a/.wolf/bugs/bug-1926.md b/.wolf/bugs/bug-1926.md index 43b555ef..c4d9255e 100644 --- a/.wolf/bugs/bug-1926.md +++ b/.wolf/bugs/bug-1926.md @@ -1,16 +1,17 @@ --- id: "bug-1926" -status: "open" -status_updated: "2026-10-08" -status_note: "newly logged" +status: "fixed" +status_updated: "2026-10-09" +status_note: "" component: "scheduler" severity: "wrong-result" timestamp: "2026-10-08T16:08:34Z" last_seen: "2026-10-08T16:08:34Z" occurrences: 1 -files: ["src/scheduler/src/repl_break.c"] +files: ["src/scheduler/src/repl_break.c", "src/scheduler/src/repl_fmt.c", "src/scheduler/src/repl_internal.h", "src/scheduler/src/repl_trace_store.c", "src/scheduler/testsuite/test_repl_breakpoints_inspection.c"] tags: ["sim", "repl", "breakpoint", "jobid"] -related: [] +related: ["bug-1930", "bug-1591"] +fix_commits: ["7aef6b85"] gitea_issue: 351 gitea_synced_status: "open" gitea_synced_hash: "923897c0e32d" @@ -52,15 +53,34 @@ Suspected: `repl_u64_str_eq()` (src/scheduler/src/repl_break.c:210) formats the - Evidence: `/home/claude/tmp/bug-1926/` (`repro.sh`, `repro.out`) ## Root cause -TODO: required for fixed/partial +`repl_u64_str_eq()` (src/scheduler/src/repl_break.c) rendered the packed 64-bit jobid with `%llu` and `strcmp()`ed it against the filter's text, so a `jobid=` filter matched only when the operator typed the raw packed decimal id. Every jobid filter went through it: `repl_decision_matches_filter()` (DECISION) and `repl_event_matches_filter()` (JOB_STATE_CHANGE, JOB_UPDATE, JOB_RUSAGE, and each job of a JOB_PUSH). The REPL prints jobids with `fmt_jobid()` (`0-0-0-104`, `X[k]` for array elements, since bug-1591), and `info job`/`info trace`/`trace on job` parse that form, so an id copied from the REPL output never compared equal. `repl_cmd_break()` stored the value as an uninterpreted string and never checked it, so nothing told the operator the breakpoint could not fire. + +Since: commit 2fecea35 (scheduler-core task 12.2, 2026-08-07), which added `repl_u64_str_eq`; it became visible once bug-1591 switched the REPL's own output to the dashed form. + +Scope: checked every caller of `repl_u64_str_eq` (the four event cases and the decision case above; there were no others). `state=`, `type=` and `queue=` filters compare names, not jobids, and are unaffected. `info job` (`parse_jobid`), `info trace` and `trace on job` (`repl_parse_jobsel_arg`, bug-1930) already parsed jobids. ## Fix -TODO: required for fixed/partial +- `repl_parse_jobsel_arg()` moved from src/scheduler/src/repl_trace_store.c to src/scheduler/src/repl_fmt.c (declaration stays in repl_internal.h), next to a new `repl_jobsel_matches(sel, raw, jobid)`. repl_fmt.c holds the REPL's pure shared helpers and is linked into every build that has repl_break.c or the trace store, so breakpoints use the jobid parser without depending on the trace store. Leaving it in repl_trace_store.c would have made the older breakpoint module depend on the trace store. +- `xce_repl_filter_t` (repl_internal.h) gains `jobsel` and `jobsel_raw`. `repl_cmd_break()` (repl_break.c) parses a `jobid=` value once, when the breakpoint is set. A value the parser rejects prints `[sim] break: invalid jobid '': ` and no breakpoint is created, like a malformed filter. `value` keeps the text as typed, so the "breakpoint N set" line and `info break` still echo what the operator wrote. +- `repl_u64_str_eq()` is replaced by `repl_filter_jobid_matches()`, which calls `repl_jobsel_matches()` on the parsed selector. The event and decision filter functions take the filter struct instead of key/value strings. So every `jobid=` filter, event or decision, uses the same parse and the same matching. +- Matching semantics (repl_jobsel_matches): a raw packed decimal id matches exactly that id, as before the fix; `X[k]` matches element k of array X; `X[a-b]` matches elements a..b; a bare dashed id matches the plain job with that id and, when the id is array-aligned, any element of the array with that id. That is the set `trace on job` selects (scheduler-sim-decision-trace Requirements 1.2/1.3, tasks.md Implementation Note 1.3). The raw form keeps exact-id matching, not the bare-id array widening, so existing scripts behave exactly as before. The packed id of an element carries the array tag, so a raw element id still names that one element. +- Rejected alternatives: re-rendering the jobid with `fmt_jobid()` and comparing strings would not cover `X[a-b]` ranges or a bare array id, and would still accept values that can never match. Parsing at match time would re-parse on every event and decision, and could not report a bad value, because the match runs inside the driver loop with no error channel. +- Behaviour change: a `jobid=` value that is neither a jobid form nor a decimal (e.g. `jobid=bogus`, `jobid=`, a reversed range) is now refused. No committed test or script relied on that: they all use raw decimal ids (`12345`, `42`, `222`, `105`, the 1.1 baseline script's `104`). `help` does not document the filter value format, so it is unchanged. ## Verification -- Test: TODO: name + file; RED before the fix, GREEN after -- Suites: TODO: suite results and build profile -- Live: TODO: soak/deploy run, or n/a and why +- Test: test_repl_breakpoints_inspection.c (sched_repl_breakpoints_inspection_test), 7 new tests. RED before the fix, 40 tests 6 failures (`/home/claude/tmp/bug-1926/unit.red.log`): + ``` + test_decision_breakpoint_jobid_dashed_form_matches:FAIL: jobid=0-0-0-104 must match job 104 + test_decision_breakpoint_jobid_element_and_range_match:FAIL: X[2] must match element 2 + test_decision_breakpoint_jobid_bare_array_id_and_raw:FAIL: a bare id matches the plain job + test_break_invalid_jobid_rejected:FAIL: Expected 0 Was 1. break DECISION type=PEND jobid=bogus + test_event_breakpoint_jobid_dashed_form_matches:FAIL: Expected Non-NULL + test_decision_breakpoint_dashed_jobid_pauses_run:FAIL: (captured run: "PEND jobid=0-0-0-555" printed, no "hit breakpoint 1") + ``` + `test_info_break_echoes_jobid_filter_as_typed` passed before and after; it guards the listing. GREEN after the fix: 40 tests 0 failures (`unit.green.log`). The tests cover dashed decision and event (JOB_STATE_CHANGE, JOB_UPDATE, JOB_RUSAGE, JOB_PUSH) filters, `X[k]` copied from `fmt_jobid()`, `X[a-b]`, a bare array id (plain job and elements), raw decimal (exact, including an element's packed id), nine rejected values, and an in-process `sched_sim_driver_run()` with a dashed decision breakpoint. +- Suites: `bash src/tools/xce-build.sh scheduler --jobs 12 --incremental` (asan, src/xce.build): scheduler suite 118/118 OK, 0 fail (`/home/claude/tmp/bug-1926/gate.log`). It includes the repl trace baseline test: the committed 1.1 transcript is unchanged, since its script uses the raw `jobid=104`. Also the trace e2e, trace cmd, trace store and sim isolation tests. +- Live: real `xce_scheduler --sim --interactive` binary on the committed contention fixture. Before (`repro.before.out`, the pre-fix binary at logging time): `jobid=0-0-0-104` printed only "breakpoint 1 set", and only `jobid=104` hit. After (`repro.after.out`, `sh repro.sh `): both `jobid=0-0-0-104` and `jobid=104` print `[sim] hit breakpoint 1 @ decision: PEND jobid=0-0-0-104`. `repro-forms.sh` (`repro-forms.after.out`): `break JOB_STATE_CHANGE jobid=0-0-0-102` hits at event #6; `break JOB_PUSH jobid=0-0-0-108` hits at event #7; `jobid=0-0-0-104[` and `jobid=bogus` are refused with `[sim] break: invalid jobid '...'`, and `info break` then reports no breakpoints; `info break` lists `jobid=0-0-0-105 type=DISPATCH` and `jobid=104` as typed. All evidence is in `/home/claude/tmp/bug-1926/`. ## References -- Related: none +- Related: bug-1930 (introduced the shared `repl_parse_jobsel_arg()` this fix reuses), bug-1591 (switched REPL jobid output to the dashed form) +- Docs: .kiro/specs/scheduler-sim-decision-trace/tasks.md (Implementation Note 1.3, bare-id semantics), src/scheduler/src/repl_internal.h (`repl_jobsel_matches` contract)