Skip to content

LoopState: 'inProgress' is loaded from disk and round-tripped to writes, but never consulted to influence behavior #90

@joewalker

Description

@joewalker

Observed behavior

LoopState stores a private #inProgress field that is:

  1. Loaded from the persisted JSON in LoopState.create():

    return new LoopState(
      path,
      data.completed ?? [],
      data.failed ?? [],
      data.inProgress,
    );
  2. 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:

  1. 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
  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    S4Clean-ups or nits with low behavioral riskbugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions