docs(scheduler-sim-reconcile-terminal-guard): add approved spec (requirements/design/tasks)
Formalizes the fix for bug-1764 through this project's TDD process: sim_reconcile_unaccounted_jobs() (sim_loop.c:664) triggers on job->accounts == NULL alone, with no check on job->state -- a deliberate widening from scheduler-sim-eager-reconciliation to catch a PEND job's early-recorded DISPATCH before its RUN confirmation replays, but one that never excluded jobs already at a terminal state (DONE/EXIT). Since xce_core_push_events() correctly frees allocs/ accounts on a terminal transition (leaving accounts == NULL, the same trigger signal), a later round's leftover/duplicate DISPATCH entry for that job id gets misread as something to reconcile, resurrecting the allocation on an already-dead job -- directly observable via info worker's assigned-jobs listing showing a completed job as still consuming a worker's resources. Requirement 1 adds a terminal-state exclusion to the trigger check, mirroring job_state_regresses_terminal()'s existing "terminal is permanent" invariant (core_events.c). Requirement 2 captures the resulting REPL-visible outcome without requiring any REPL code change. The JOBS_PUSH resync-merge branch (core_events.c, bug-1734/bug-1738) was investigated during design and confirmed to already guard against this exact problem -- explicitly out of this spec's scope. Generated via kiro-spec-quick with an inline requirements review gate, an inline design review gate, and an independent task-graph sanity review (one NEEDS_FIXES round, repaired: task 2.2's REPL-level verification mechanism was underspecified, now pins down the exact capture_dispatch()-driven harness and stray-DISPATCH-injection technique to use). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YAWMhfRJFnmmnG8h97yUDd
This commit is contained in:
parent
56bd3103f9
commit
f854d9facd
5 changed files with 245 additions and 0 deletions
137
.kiro/specs/scheduler-sim-reconcile-terminal-guard/design.md
Normal file
137
.kiro/specs/scheduler-sim-reconcile-terminal-guard/design.md
Normal file
|
|
@ -0,0 +1,137 @@
|
||||||
|
# Design Document
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
`sim_reconcile_unaccounted_jobs()` (`sim_loop.c:664`) triggers reconciliation for any job with `job->accounts == NULL` (line 690), deliberately state-independent since `scheduler-sim-eager-reconciliation` widened it to catch a PEND job's early-recorded DISPATCH before its RUN confirmation replays. `xce_core_push_events()`'s existing end-of-life cleanup (`core_events.c:1127-1135` for `JOB_STATE_CHANGE`, `:1179-1186` for `JOB_UPDATE`) already frees `job->allocs`/`job->accounts` on a terminal transition — which leaves `job->accounts == NULL`, the exact same trigger condition. If a later round's `decoded_session->decisions` still carries an unconsumed DISPATCH entry for that job id, reconciliation matches it and resurrects the allocation on an already-dead job, with nothing to stop it (bug-1764).
|
||||||
|
|
||||||
|
This feature adds one additional exclusion to the trigger check: a job whose current state is already terminal (DONE or EXIT) is never considered for reconciliation, regardless of whether `job->accounts == NULL` or a matching DISPATCH entry exists. Every other aspect of the function — the eager-capture behavior for non-terminal jobs, the decision-reporting sequence, the array-compaction/ownership-transfer logic in the FOUND branch — is unchanged.
|
||||||
|
|
||||||
|
**Users**: Operators reading `info worker`'s assigned-jobs listing or `info summary`'s running/pending counts through the interactive REPL, and any future consumer of `job->allocs`/`job->accounts`'s correctness.
|
||||||
|
**Impact**: `sim_reconcile_unaccounted_jobs()` gains one additional condition in its existing trigger check; no other function changes.
|
||||||
|
|
||||||
|
### Goals
|
||||||
|
- A job that has reached DONE or EXIT is never re-allocated by reconciliation, no matter what the replay stream still contains for it.
|
||||||
|
- The existing eager-capture behavior for PEND/RUN jobs is completely unaffected.
|
||||||
|
- The fix reuses this codebase's own already-established "terminal is permanent" pattern rather than inventing a new one.
|
||||||
|
|
||||||
|
### Non-Goals
|
||||||
|
- Changing how or when a job's state actually transitions to DONE/EXIT.
|
||||||
|
- Changing `repl_info.c`'s assigned-jobs rendering — it is a correct consumer of `job->allocs`, not the defect.
|
||||||
|
- Changing the JOBS_PUSH resync-merge branch (`core_events.c`, bug-1734/bug-1738 lines ~822-901) — investigated during design (see Existing Architecture Analysis) and confirmed to already guard against this exact class of problem via `job_state_regresses_terminal()`; left unmodified.
|
||||||
|
- Investigating bug-1611 (the separate, still-open question of why fresh DISPATCH decisions are rare in replay mode) — unrelated to this fix, which only prevents an EXISTING duplicate/leftover DISPATCH entry from being misapplied.
|
||||||
|
|
||||||
|
## Boundary Commitments
|
||||||
|
|
||||||
|
### This Spec Owns
|
||||||
|
- The terminal-state exclusion added to `sim_reconcile_unaccounted_jobs()`'s trigger check (`sim_loop.c`).
|
||||||
|
- A regression test in `test_sim_reconcile_unaccounted.c` proving a terminal job's allocation is never resurrected by a stray recorded DISPATCH entry.
|
||||||
|
|
||||||
|
### Out of Boundary
|
||||||
|
- `repl_info.c`'s assigned-jobs listing — unmodified; its correctness is a direct, automatic consequence of this fix, not something it implements itself.
|
||||||
|
- `xce_core_push_events()`'s JOB_STATE_CHANGE/JOB_UPDATE terminal-transition cleanup, and its JOBS_PUSH resync-merge branch's own terminal-state guard (`core_events.c`) — both confirmed correct, neither modified.
|
||||||
|
- `sched_commit_decision()` (`scheduler.c`) and the live-mode dispatch path — untouched; this fix is confined to the simulation driver's replay-reconciliation path.
|
||||||
|
|
||||||
|
### Allowed Dependencies
|
||||||
|
- `JOB_IS_DONE()`/`JOB_IS_EXIT()` (`xce/types.h`, existing) — the same state-testing macros `job_state_regresses_terminal()` (`core_events.c:696-701`) already uses for the analogous "terminal is permanent" rule this fix mirrors.
|
||||||
|
- `xce_sched_job_t.state` (existing field) — read-only from this fix's perspective; never written by `sim_reconcile_unaccounted_jobs()`.
|
||||||
|
|
||||||
|
### Revalidation Triggers
|
||||||
|
- If a new job-lifecycle terminal state is ever introduced beyond DONE/EXIT, this exclusion and `job_state_regresses_terminal()` must both be re-checked together — they now encode the same invariant in two places.
|
||||||
|
- If `sim_reconcile_unaccounted_jobs()`'s trigger condition is ever restructured (e.g. keyed on something other than `job->accounts == NULL`), re-verify the terminal-state exclusion still composes correctly with the new trigger.
|
||||||
|
|
||||||
|
## Architecture
|
||||||
|
|
||||||
|
### Existing Architecture Analysis
|
||||||
|
`sim_reconcile_unaccounted_jobs()`'s current trigger (line 690) is a single `if` guarding the per-job loop body: `if ((job == NULL) || (job->accounts != NULL)) { continue; }`. This is the ONLY place in the function that decides eligibility; the FOUND branch below it (array search, ownership transfer, decision reporting) runs unconditionally once a job passes this gate. Adding the terminal-state exclusion here — rather than inside the FOUND branch, after a match is already located — is both minimal and correct: a terminal job should never even be searched for a match, since any match found for it would necessarily be the exact stale/duplicate entry this fix exists to ignore.
|
||||||
|
|
||||||
|
`job_state_regresses_terminal()` (`core_events.c:696-701`) already encodes "once DONE or EXIT, a state change to anything else is rejected" for the JOB_STATE_CHANGE event-application path and the JOBS_PUSH resync-merge path. This fix does not call that function (it is a state-CHANGE guard, not an allocation guard, and lives in a different file/domain); instead it applies the equivalent `JOB_IS_DONE(job->state) || JOB_IS_EXIT(job->state)` test directly, matching the same semantic rule at the point this fix actually needs it.
|
||||||
|
|
||||||
|
Investigated during design, per this feature's Non-Goals: the JOBS_PUSH resync-merge branch (`core_events.c`, the `RUN`-merge and non-`RUN`-merge sub-branches around lines 822-901) explicitly calls `job_state_regresses_terminal(existing->state, job->state)` before ever touching `existing->allocs`/`existing->accounts` — confirmed already safe for this exact class of problem. No change needed there.
|
||||||
|
|
||||||
|
**Architecture Integration**:
|
||||||
|
- Selected pattern: extend one existing function's own trigger condition; no new component, no new file.
|
||||||
|
- Domain/feature boundaries: strictly inside the simulation driver's Recorded-Decision Reconciliation component (`scheduler-sim-decision-parity`/`scheduler-sim-eager-reconciliation`'s existing domain, `sim_loop.c`); no other domain touched.
|
||||||
|
- Existing patterns preserved: `JOB_IS_DONE()`/`JOB_IS_EXIT()` as the standard terminal-state test; "terminal is permanent" as an invariant already established and now applied consistently at a second enforcement point.
|
||||||
|
- New components rationale: none — a one-condition change to an existing trigger check.
|
||||||
|
- Steering compliance: `src/scheduler/CLAUDE.md`'s 800-line-per-file cap — `sim_loop.c`'s current size should be checked before landing; this change adds at most a few lines to one existing `if` condition plus doc-comment updates, not expected to be a meaningful contributor either way.
|
||||||
|
|
||||||
|
## File Structure Plan
|
||||||
|
|
||||||
|
### Modified Files
|
||||||
|
- `src/scheduler/src/sim_loop.c` — `sim_reconcile_unaccounted_jobs()`'s trigger check gains a terminal-state exclusion: `if ((job == NULL) || (job->accounts != NULL) || JOB_IS_DONE(job->state) || JOB_IS_EXIT(job->state)) { continue; }`. The function's own doc comment (lines 589-608) is updated to state the terminal exclusion alongside the existing state-independence rationale, so a future reader sees both rules together rather than the doc comment appearing to contradict the code.
|
||||||
|
|
||||||
|
### New/Modified Test Files
|
||||||
|
- `src/scheduler/testsuite/test_sim_reconcile_unaccounted.c` (extend, existing home for this function's tests) — a new regression test mirroring the reverted probe that originally found this bug: drive a job to `JOB_STAT_DONE` via `xce_core_push_events()` (confirming its `allocs`/`accounts` are correctly nulled by the existing cleanup), then call `sim_reconcile_unaccounted_jobs()` with a `decoded_session` carrying a DISPATCH entry for that same job id, and assert `allocs`/`accounts` remain NULL afterward. A second test confirms the existing eager-capture case (a PEND job's early DISPATCH entry) still succeeds exactly as before, guarding against a regression in the opposite direction.
|
||||||
|
|
||||||
|
## Requirements Traceability
|
||||||
|
|
||||||
|
| Requirement | Summary | Components | Interfaces | Flows |
|
||||||
|
|-------------|---------|------------|------------|-------|
|
||||||
|
| 1.1 | Terminal job never re-allocated by reconciliation | Terminal-State Guard | `sim_reconcile_unaccounted_jobs()` | n/a |
|
||||||
|
| 1.2 | Non-terminal eager-capture unaffected | Terminal-State Guard | `sim_reconcile_unaccounted_jobs()` | n/a |
|
||||||
|
| 1.3 | Terminal state treated as permanent, consistent with existing invariant | Terminal-State Guard | `sim_reconcile_unaccounted_jobs()` | n/a |
|
||||||
|
| 2.1 | Assigned-jobs listing never shows a terminal job | Out of Boundary (automatic consequence, `repl_info.c` unmodified) | n/a | n/a |
|
||||||
|
| 2.2 | No REPL code change required | Out of Boundary | n/a | n/a |
|
||||||
|
|
||||||
|
## Components and Interfaces
|
||||||
|
|
||||||
|
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|
||||||
|
|-----------|--------------|--------|---------------|---------------------------|-----------|
|
||||||
|
| Terminal-State Guard | Scheduler Core / sim_loop.c | Exclude a terminal job from reconciliation's trigger check | 1.1, 1.2, 1.3 | `JOB_IS_DONE()`/`JOB_IS_EXIT()` (P0) | Service |
|
||||||
|
|
||||||
|
### Scheduler Core (sim_loop.c)
|
||||||
|
|
||||||
|
#### Terminal-State Guard
|
||||||
|
| Field | Detail |
|
||||||
|
|-------|--------|
|
||||||
|
| Intent | Prevent `sim_reconcile_unaccounted_jobs()` from populating or modifying a terminal job's allocation/accounting state |
|
||||||
|
| Requirements | 1.1, 1.2, 1.3 |
|
||||||
|
|
||||||
|
**Responsibilities & Constraints**
|
||||||
|
- Adds exactly one condition to the existing per-job trigger check (line 690): a job whose state is `JOB_IS_DONE()` or `JOB_IS_EXIT()` is skipped before the DISPATCH-entry search runs, regardless of `job->accounts`'s value.
|
||||||
|
- Does not alter `job->state`, `job->allocs`, or `job->accounts` for a terminal job in any way — the guard's only effect is to leave the loop body entirely, exactly like the existing `job->accounts != NULL` skip already does for a non-terminal, already-captured job.
|
||||||
|
- Does not alter the FOUND branch's array-compaction, ownership-transfer, or decision-reporting logic for a non-terminal job — those paths are reached identically to today whenever a non-terminal job passes the (now three-part) trigger check.
|
||||||
|
|
||||||
|
**Dependencies**
|
||||||
|
- Inbound: `sched_sim_driver_run()`'s round-boundary call site (`sim_loop.c`, existing, unmodified) (P0)
|
||||||
|
- Outbound: `JOB_IS_DONE()`, `JOB_IS_EXIT()` (`xce/types.h`, existing) (P0)
|
||||||
|
|
||||||
|
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
|
||||||
|
|
||||||
|
##### Service Interface
|
||||||
|
```c
|
||||||
|
/* sim_reconcile_unaccounted_jobs() -- signature unchanged. Behavioral
|
||||||
|
* contract addition: a job whose state is JOB_IS_DONE() or JOB_IS_EXIT()
|
||||||
|
* is now guaranteed to leave this call with its allocs/accounts fields
|
||||||
|
* bit-for-bit unchanged from whatever they were on entry, regardless of
|
||||||
|
* decoded_session's contents. */
|
||||||
|
static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
|
||||||
|
replay_session_t* decoded_session,
|
||||||
|
sched_record_session_ctx_t* record_ctx);
|
||||||
|
```
|
||||||
|
- Preconditions: unchanged from today (`sim`/`decoded_session` NULL-safe no-op; `record_ctx` may legitimately be NULL).
|
||||||
|
- Postconditions: a terminal job's `allocs`/`accounts` are identical before and after the call; a non-terminal job's behavior is identical to today's.
|
||||||
|
- Invariants: a job's terminal state, once reached, is never overridden by this function — mirrors `job_state_regresses_terminal()`'s existing invariant in `core_events.c`.
|
||||||
|
|
||||||
|
**Implementation Notes**
|
||||||
|
- Integration: no change to this function's callers, signature, or the decision-reporting sequence in its FOUND branch.
|
||||||
|
- Validation: covered by the new regression test plus the existing `test_sim_reconcile_unaccounted.c` suite's pre-existing eager-capture tests, which must continue to pass unmodified.
|
||||||
|
- Risks: none beyond the small `sim_loop.c` growth already addressed in Architecture Integration above.
|
||||||
|
|
||||||
|
## Error Handling
|
||||||
|
|
||||||
|
### Error Strategy
|
||||||
|
No new failure modes: this feature adds a pure state comparison to an existing conditional — no allocation, no I/O, no new error path.
|
||||||
|
|
||||||
|
### Monitoring
|
||||||
|
No new logging — this is a silent skip, matching the function's existing silent skip for an already-captured (`job->accounts != NULL`) job; no operator-facing signal is needed since Requirement 2's observable outcome (a correct assigned-jobs listing) is itself the confirmation.
|
||||||
|
|
||||||
|
## Testing Strategy
|
||||||
|
|
||||||
|
### Unit Tests (extend `test_sim_reconcile_unaccounted.c`)
|
||||||
|
- A job driven to `JOB_STAT_DONE` via `xce_core_push_events()` (confirming its `allocs`/`accounts` are nulled by the existing cleanup), then presented with a `decoded_session` carrying a DISPATCH entry for that job id: `sim_reconcile_unaccounted_jobs()` leaves `allocs`/`accounts` NULL (1.1).
|
||||||
|
- The same scenario with `JOB_STAT_EXIT` instead of `JOB_STAT_DONE` (1.1, 1.3 — both terminal states must be covered, not just one).
|
||||||
|
- A PEND job's early-recorded DISPATCH entry is still captured exactly as before (regression guard for 1.2 — must continue passing unmodified from the existing suite, and a new test if the existing suite doesn't already cover this exact "not yet RUN, gets captured" shape explicitly).
|
||||||
|
|
||||||
|
### Integration Tests (existing suites, re-run, no new tests required)
|
||||||
|
- `test_sim_e2e_original_motivation.c` / `test_repl_resource_table.c`'s `info worker` assigned-jobs tests: re-run to confirm no regression (Requirement 2's observable outcome; no new test needed since these already exercise the real display path against real job state).
|
||||||
|
- Full scheduler test suite: confirm no regression anywhere else, matching the discipline already established in this session's other specs.
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
# Requirements Document
|
||||||
|
|
||||||
|
## Introduction
|
||||||
|
The simulation driver's decision-reconciliation logic (`sim_reconcile_unaccounted_jobs()`) exists to catch a job whose DISPATCH decision was recorded in a replay round before that job's RUN confirmation has been decoded — a deliberate, state-independent widening introduced by `scheduler-sim-eager-reconciliation` so an early-recorded dispatch is never missed. That widening did not account for jobs that have since moved all the way to a terminal state (DONE or EXIT): once a job completes, its resource allocation is correctly released, but a later round's replay stream can still carry a leftover or duplicate DISPATCH entry for that same job id (for example, from an array job with retried dispatch attempts). Because reconciliation currently triggers on "no allocation recorded" alone, it cannot distinguish "not yet allocated" from "already completed and released," and resurrects the allocation on the completed job. This is directly observable through `info worker`'s assigned-jobs listing, which then shows a job as still consuming a worker's resources long after it finished — bug-1764, confirmed via a live interactive session and independently reproduced with a targeted probe. This feature closes that gap: reconciliation must never act on a job that has already reached a terminal state, while continuing, unchanged, to capture the early-dispatch case for jobs that have not yet terminated.
|
||||||
|
|
||||||
|
## Boundary Context
|
||||||
|
- **In scope**: adding a terminal-state check to the simulation driver's reconciliation logic (`sim_reconcile_unaccounted_jobs()`) so it never populates or modifies a job's allocation/accounting state once that job has reached DONE or EXIT.
|
||||||
|
- **Out of scope**: any change to how `info worker`/`info summary` render a job's assignment (the display code is a correct, unmodified consumer of the job's actual allocation state — the defect is upstream of it); any change to how a job's state actually transitions to DONE/EXIT; any change to the replay/decision-stream format; the JOBS_PUSH resync-merge branch's own terminal-state handling in `core_events.c`, which is believed to already be correct for an analogous problem and is confirmed, not modified, by this feature unless a genuine gap is found (in which case it is reported, not silently folded into this fix).
|
||||||
|
- **Adjacent expectations**: this feature depends on the job-state-change event handling that already frees a job's allocation on a terminal transition continuing to behave exactly as it does today — this feature does not change when or how a job's allocation is released, only what reconciliation is allowed to do to a job whose allocation is already gone because it terminated.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
### Requirement 1: Terminal-State Guard on Job Reconciliation
|
||||||
|
**Objective:** As an operator relying on the simulation driver's internal job/resource bookkeeping to reflect reality, I want reconciliation logic to never re-establish a resource allocation for a job that has already finished, so that a completed job can never be treated as still holding resources.
|
||||||
|
|
||||||
|
#### Acceptance Criteria
|
||||||
|
1. When the simulation driver's reconciliation logic considers a job whose current state is DONE or EXIT, the simulation driver shall not populate or modify that job's allocation or accounting state, regardless of whether a matching recorded dispatch entry exists in the replay stream.
|
||||||
|
2. While a job's state is not terminal, the simulation driver's reconciliation logic shall continue to capture a matching recorded dispatch entry for that job exactly as it does today, unaffected by this feature.
|
||||||
|
3. The simulation driver shall treat a job's terminal state as permanent for reconciliation purposes, consistent with how terminal states are already treated as permanent elsewhere in this driver's job-state handling.
|
||||||
|
|
||||||
|
### Requirement 2: Assigned-Resource Display Consistency
|
||||||
|
**Objective:** As an operator reading `info worker`'s assigned-jobs listing, I want it to never show a job that has already completed, so the listing stays consistent with the running/pending counts `info summary` reports.
|
||||||
|
|
||||||
|
#### Acceptance Criteria
|
||||||
|
1. When a job has reached a terminal state, the REPL's assigned-jobs listing for the worker that job was dispatched to shall not include that job, even if a later replay round's decoded session carries a recorded dispatch entry for that job id.
|
||||||
|
2. The REPL shall require no code change of its own to satisfy this requirement — the listing already derives its content solely from the job's actual allocation state, and shows only what Requirement 1 keeps correct.
|
||||||
|
|
@ -0,0 +1,35 @@
|
||||||
|
# Research & Design Decisions
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
- **Feature**: `scheduler-sim-reconcile-terminal-guard`
|
||||||
|
- **Discovery Scope**: Extension (bug fix inside an existing component, `scheduler-sim-eager-reconciliation`'s Recorded-Decision Reconciliation)
|
||||||
|
- **Key Findings**:
|
||||||
|
- The bug's root cause and exact fix point were already pinned down before this spec was created, via a live reproduction and a temporary, fully-reverted probe test (see `.wolf/buglog.json` bug-1764) — this spec formalizes and completes that investigation through the project's TDD process rather than re-deriving it.
|
||||||
|
- The analogous JOBS_PUSH resync-merge branch (`core_events.c`, bug-1734/bug-1738) was checked during design and confirmed to already guard against this exact class of problem via `job_state_regresses_terminal()` — no gap found there, so it is explicitly out of this spec's scope rather than silently expanded into.
|
||||||
|
|
||||||
|
## Research Log
|
||||||
|
|
||||||
|
### Does the JOBS_PUSH resync-merge branch have the same gap?
|
||||||
|
- **Context**: bug-1764's own fix-direction note flagged this as worth checking before finalizing scope.
|
||||||
|
- **Sources Consulted**: `core_events.c`'s `XCE_CORE_EVENT_JOBS_PUSH` case, specifically the resync-merge branch handling a duplicate jobid (~lines 822-901).
|
||||||
|
- **Findings**: This branch explicitly calls `job_state_regresses_terminal(existing->state, job->state)` before merging a resync redelivery's state into an existing tracked job, and refuses the merge (including any `allocs`/`accounts` transfer) when the existing job is already terminal and the redelivery would regress it — the exact protection bug-1764 is missing from `sim_reconcile_unaccounted_jobs()`.
|
||||||
|
- **Implications**: Confirmed no analogous gap exists there; design.md's Non-Goals explicitly excludes this branch from this spec's scope, with the confirmation recorded here rather than left as an open question.
|
||||||
|
|
||||||
|
## Design Decisions
|
||||||
|
|
||||||
|
### Decision: Guard at the trigger check, not inside the FOUND branch
|
||||||
|
- **Context**: the terminal-state exclusion could be added either at line 690's trigger check (before searching for a match) or inside the FOUND branch (after a match is located, before acting on it).
|
||||||
|
- **Alternatives Considered**:
|
||||||
|
1. Guard inside the FOUND branch, after the search — would still perform an unnecessary array scan for every terminal job, every round, for the rest of the replay.
|
||||||
|
2. Guard at the trigger check — **selected**. A terminal job is excluded before any search happens, matching the existing `job->accounts != NULL` skip's own placement and semantics exactly (both are "this job is not eligible for reconciliation at all" conditions, not "this job might have a match but we should reject it").
|
||||||
|
- **Selected Approach**: `JOB_IS_DONE(job->state) || JOB_IS_EXIT(job->state)` added as an additional disjunct in the existing trigger `if`.
|
||||||
|
- **Rationale**: Minimal, consistent with the existing code's own structure, and avoids wasted per-round work on jobs that can never legitimately match again.
|
||||||
|
- **Trade-offs**: None material.
|
||||||
|
- **Follow-up**: None.
|
||||||
|
|
||||||
|
## Risks & Mitigations
|
||||||
|
- Risk: a future terminal state beyond DONE/EXIT is added without updating this guard — mitigated by design.md's Revalidation Triggers explicitly calling out that this exclusion and `job_state_regresses_terminal()` now encode the same invariant in two places and must be updated together.
|
||||||
|
|
||||||
|
## References
|
||||||
|
- `.wolf/buglog.json` bug-1764 — the original root-cause investigation and reproduction this spec formalizes.
|
||||||
|
- `.kiro/specs/scheduler-sim-eager-reconciliation/` — the spec that introduced the state-independent trigger widening this fix narrows for the terminal case only.
|
||||||
22
.kiro/specs/scheduler-sim-reconcile-terminal-guard/spec.json
Normal file
22
.kiro/specs/scheduler-sim-reconcile-terminal-guard/spec.json
Normal file
|
|
@ -0,0 +1,22 @@
|
||||||
|
{
|
||||||
|
"feature_name": "scheduler-sim-reconcile-terminal-guard",
|
||||||
|
"created_at": "2026-09-03T06:32:45Z",
|
||||||
|
"updated_at": "2026-09-03T07:05:00Z",
|
||||||
|
"language": "en",
|
||||||
|
"phase": "tasks-generated",
|
||||||
|
"approvals": {
|
||||||
|
"requirements": {
|
||||||
|
"generated": true,
|
||||||
|
"approved": true
|
||||||
|
},
|
||||||
|
"design": {
|
||||||
|
"generated": true,
|
||||||
|
"approved": true
|
||||||
|
},
|
||||||
|
"tasks": {
|
||||||
|
"generated": true,
|
||||||
|
"approved": true
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"ready_for_implementation": true
|
||||||
|
}
|
||||||
25
.kiro/specs/scheduler-sim-reconcile-terminal-guard/tasks.md
Normal file
25
.kiro/specs/scheduler-sim-reconcile-terminal-guard/tasks.md
Normal file
|
|
@ -0,0 +1,25 @@
|
||||||
|
# Implementation Plan
|
||||||
|
|
||||||
|
- [ ] 1. Core: terminal-state guard on job reconciliation
|
||||||
|
- [ ] 1.1 Write failing tests for the terminal-state guard
|
||||||
|
- Cover: a job driven to JOB_STAT_DONE via xce_core_push_events() (confirming allocs/accounts are nulled by the existing cleanup), then presented with a decoded_session carrying a DISPATCH entry for that job id, must leave allocs/accounts NULL after sim_reconcile_unaccounted_jobs() runs; the same scenario with JOB_STAT_EXIT instead of JOB_STAT_DONE; a PEND job's early-recorded DISPATCH entry is still captured exactly as today (explicit regression guard, not just relying on the pre-existing suite)
|
||||||
|
- Observable: the new test cases for DONE/EXIT fail against today's sim_reconcile_unaccounted_jobs(), which resurrects allocs/accounts on any job with accounts == NULL regardless of state
|
||||||
|
- _Requirements: 1.1, 1.2, 1.3_
|
||||||
|
- _Boundary: Terminal-State Guard_
|
||||||
|
|
||||||
|
- [ ] 1.2 Implement the terminal-state exclusion in sim_reconcile_unaccounted_jobs()
|
||||||
|
- Add JOB_IS_DONE(job->state) || JOB_IS_EXIT(job->state) as an additional exclusion in the function's existing per-job trigger check, alongside the existing job->accounts != NULL skip; update the function's own doc comment to state the terminal exclusion alongside its existing state-independence rationale so the two don't read as contradictory
|
||||||
|
- Observable: all tests from 1.1 pass; a terminal job's allocs/accounts are provably unchanged before and after a call that presents a matching stray DISPATCH entry for it
|
||||||
|
- _Requirements: 1.1, 1.2, 1.3_
|
||||||
|
- _Boundary: Terminal-State Guard_
|
||||||
|
|
||||||
|
- [ ] 2. Validation: regression and display-consistency verification
|
||||||
|
- [ ] 2.1 Verify no regression to existing reconciliation behavior across the scheduler test suite
|
||||||
|
- Confirm the full scheduler test suite passes with no new failures beyond any already-documented pre-existing ones, and that the pre-existing eager-capture tests in test_sim_reconcile_unaccounted.c and the JOBS_PUSH resync-merge tests in test_core_events.c continue to pass unmodified
|
||||||
|
- Observable: full test suite result showing no regression, with the pre-existing eager-capture and resync-merge test names explicitly confirmed still passing
|
||||||
|
- _Requirements: 1.2_
|
||||||
|
|
||||||
|
- [ ] 2.2 Re-verify against a realistic array-job scenario that info worker's assigned-jobs listing no longer shows a completed job
|
||||||
|
- Using the same capture_dispatch()-driven REPL test harness already established in test_sim_e2e_original_motivation.c/test_repl_resource_table.c, construct a multi-job scenario mirroring bug-1764's live-observed shape: an array job where some elements are driven to JOB_STAT_DONE/JOB_STAT_EXIT via xce_core_push_events() (as in 1.1's tests) while others remain running, then advance the sim driver through a further round whose decoded_session carries a stray/duplicate DISPATCH entry (built the same way as 1.1's regression tests) for one of the now-terminal job ids. Issue info worker <host> and info summary through capture_dispatch() and assert the completed job is absent from the assigned-jobs listing, the still-running jobs are present, and the two commands' counts agree
|
||||||
|
- Observable: a captured info worker transcript showing the completed job absent from the assigned-jobs listing, with the still-running jobs present and info summary's counts consistent with it
|
||||||
|
- _Requirements: 2.1, 2.2_
|
||||||
Loading…
Add table
Add a link
Reference in a new issue