Observed behavior
LoopState stores a private #inProgress field that is:
-
Loaded from the persisted JSON in LoopState.create():
return new LoopState(
path,
data.completed ?? [],
data.failed ?? [],
data.inProgress,
);
-
Written by begin(id) and cleared by end(id, ...), with both methods calling save() so the value is round-tripped to disk.
However, nothing in LoopState or its callers ever reads the loaded value before overwriting it:
isOutstanding(id) only checks #completed and #failed. An ID that was in-progress when the previous run crashed is treated as outstanding (correct behaviour) without ever inspecting #inProgress.
begin(id) unconditionally assigns this.#inProgress = id, discarding whatever value was loaded.
- There is no getter or other public method that exposes
#inProgress.
The only place #inProgress is read after LoopState.create() is inside save(), which writes it straight back to disk. So the on-disk inProgress value persists across restarts but does not influence any decision the runtime makes.
Expected behavior
Either:
- Drop
#inProgress from the in-memory state entirely. The loop can write a transient inProgress marker through a different code path if it wants a debugging hint, and LoopState.create() should ignore the field on read; or
- Wire
#inProgress into actual behaviour, for example by exposing it (get lastInProgress(): string | undefined) so callers can log "previous run was interrupted while processing X", or by automatically marking the loaded #inProgress ID as a known-suspicious retry.
As written, the field reads like deliberately-tracked state but is functionally dead, which is a maintenance hazard. A reader cannot tell whether they are looking at intentional diagnostic data or a half-implemented feature.
Minimal reproduction
import { LoopState } from 'loop-the-loop/util/loop-state';
import { writeFile, readFile } from 'node:fs/promises';
const path = '/tmp/loop-state-inprogress.json';
await writeFile(
path,
JSON.stringify({ completed: [], failed: [], inProgress: 'crashed-item' }, null, 2),
);
const state = await LoopState.create(path);
// The loaded 'inProgress' value never influences any decision:
console.log(state.isOutstanding('crashed-item')); // true (because it's not in completed/failed)
console.log(state.isOutstanding('any-other-id')); // true
// There is no public way to observe the loaded value either.
// The next begin() simply overwrites it:
await state.begin('next-item');
console.log(JSON.parse(await readFile(path, 'utf-8')).inProgress); // 'next-item'
Suggested fix
If the field is meant as a debugging hint, document it as such and remove the constructor parameter so it cannot be misread as an input. If it is meant to drive behaviour, add the missing logic (e.g. a getter, or special-casing the loaded ID on resume) and a test that demonstrates a load-time consumer reading the value.
Observed behavior
LoopStatestores a private#inProgressfield that is:Loaded from the persisted JSON in
LoopState.create():Written by
begin(id)and cleared byend(id, ...), with both methods callingsave()so the value is round-tripped to disk.However, nothing in
LoopStateor its callers ever reads the loaded value before overwriting it:isOutstanding(id)only checks#completedand#failed. An ID that was in-progress when the previous run crashed is treated as outstanding (correct behaviour) without ever inspecting#inProgress.begin(id)unconditionally assignsthis.#inProgress = id, discarding whatever value was loaded.#inProgress.The only place
#inProgressis read afterLoopState.create()is insidesave(), which writes it straight back to disk. So the on-diskinProgressvalue persists across restarts but does not influence any decision the runtime makes.Expected behavior
Either:
#inProgressfrom the in-memory state entirely. The loop can write a transientinProgressmarker through a different code path if it wants a debugging hint, andLoopState.create()should ignore the field on read; or#inProgressinto actual behaviour, for example by exposing it (get lastInProgress(): string | undefined) so callers can log "previous run was interrupted while processing X", or by automatically marking the loaded#inProgressID as a known-suspicious retry.As written, the field reads like deliberately-tracked state but is functionally dead, which is a maintenance hazard. A reader cannot tell whether they are looking at intentional diagnostic data or a half-implemented feature.
Minimal reproduction
Suggested fix
If the field is meant as a debugging hint, document it as such and remove the constructor parameter so it cannot be misread as an input. If it is meant to drive behaviour, add the missing logic (e.g. a getter, or special-casing the loaded ID on resume) and a test that demonstrates a load-time consumer reading the value.