docs(buglog): bug-1926 fixed (7aef6b85)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7aef6b85c9
commit
6d1667c936
2 changed files with 44 additions and 19 deletions
File diff suppressed because one or more lines are too long
|
|
@ -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 '<value>': <jobsel_strerror text>` 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 <repo>`): 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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue