Skip to content

fix(goal-loop): a drain for in-flight work; tests dispose before cleanup (#1341) - #1371

Merged
Juliusolsson05 merged 6 commits into
mainfrom
fix/goal-loop-drain
Sep 27, 2026
Merged

Juliusolsson05 merged 6 commits into
mainfrom
fix/goal-loop-drain

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Closes #1341. It also covers #1296 items 3 and 7, which were moved here from #1353.

Problem

GoalLoopService started work nothing could await: void this.persist(), void this.maybeContinue(), and zero-delay setTimeout(requestContinue) deferrals.

Change

  • whenSettled() resolves when no tracked work is in flight. Tracked work is every promise the service starts without awaiting, plus every zero-delay deferral until it has run. It loops, because settling work can start more.
    • The future polls (1 s hold poll, retry backoff) are deliberately not in flight. Waiting for them would mean waiting for a hold to end.
  • dispose() stops new work (a delivery already in flight records its outcome but does not retry; defer schedules nothing; a racing start() attaches nothing; startLoop, complete and carry reject; control returns null; hooks are ignored), detaches the manager listeners start attached, cancels every timer, and drains. maybeContinue returns once disposed, so no path (including a continuation queued behind one in flight) delivers after dispose begins.
  • All raw setTimeouts go through one owned defer, so dispose can clear every timer.
  • Tests: all three goal-loop files dispose their services before removing directories. The vacuous sleeps became whenSettled(). The two sleeps that wait on real timers stay, each with the reason:
    • the 1.5 s one is a positive wait on the 1 s hold poll;
    • the 400 ms one is sound because Node fires timers in expiry order.
  • Not here (decision): wiring dispose() into the app's shutdown inventory. That composition has its own owner and exhaustive inventory, and a lost final persist at quit is pre-existing. Filed as goal-loop: dispose GoalLoopService in the app shutdown inventory (last persist can be lost at quit) #1372.
  • Plan: docs/plans/2026-09-27-goal-loop-drain.md.

Tests

  • New drain cases:
    • dispose waits for a persist parked on a gated store, then starts nothing;
    • whenSettled covers a zero-delay continuation check with no waitFor;
    • a disposed service ignores a held loop's direct quiet-edge continuation.
  • Review round 1 regressions:
    • a queued continuation behind a gated persist does not deliver once dispose begins;
    • manager events after dispose change nothing and write nothing;
    • dispose waits for a control action's persist;
    • dispose cancels a held loop's poll timer.
  • Mutations killed:
    • whenSettled resolving at once;
    • a zero-delay deferral not tracked;
    • dispose not draining;
    • a disposed service still continuing;
    • maybeContinue ignoring dispose;
    • listeners not detached;
    • control's persist untracked;
    • dispose keeping timers.
    • Round 2: the retry ignoring dispose, start ignoring dispose, defer after dispose, public mutators after dispose, control after dispose.
  • src/main/goalLoop: 113/113, with 0 persisting loop state failed warnings.
    • origin/main also logged 0 locally. The 13 warnings are from CI under load, so this PR's CI log is the real check.
  • npx tsc -b: 0 lines.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 26, 2026 23:12
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nup (#1341)

GoalLoopService fired persist/maybeContinue and zero-delay continuation
checks without anything able to await them. afterEach removed temp dirs
under a still-running write (13 ENOENT warnings on main CI run
36156560842), and fixed sleeps followed by "not delivered" passed
whenever the work had simply not run yet (#1296 items 3 and 7).

- whenSettled(): resolves when no tracked work is in flight (every
  fire-and-forget promise and every zero-delay deferral, looping because
  settling work can start more). Future polls (hold poll, retry backoff)
  are deliberately not included.
- dispose(): stops new work, cancels every timer, drains.
- All three goal-loop test files dispose their services before removing
  directories; vacuous sleeps become whenSettled(). The two sleeps that
  wait on real timers stay, with the reason.

Mutations killed: whenSettled resolving at once, a zero-delay deferral
not tracked, dispose not draining, a disposed service continuing.
106/106, 0 ENOENT warnings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 Juliusolsson05 added type:bug Something works wrong class:C9-tests Test hygiene sev:P3 Minor labels Sep 27, 2026
…und 1)

Round 1 of #1371 (a, b, c):
- a continuation replayed a queued one from its finally block straight
  into maybeContinue, past requestContinue's disposed check, and
  delivered after dispose() began: maybeContinue itself now returns when
  disposed;
- the manager listeners outlived dispose(), so a late `removed` paused
  the loop and wrote into a removed directory: start keeps them and
  dispose detaches them (`off` on the manager port).
- Pins control()'s persist as tracked work and timer cancellation.

Mutations killed: maybeContinue ignoring dispose, listeners not
detached, control persist untracked, dispose keeping timers. 110/110,
0 ENOENT warnings, tsc clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner Author

Review disposition (round 1: a, b, c codex at 4145f1cf)

# Finding Verdict Disposition
a1 / b1 / c1 A queued continuation, replayed from finally via maybeContinue, delivered after dispose() began valid Fixed: maybeContinue returns when disposed. The reviewers' sequence (gated persist, provider hooks close the turn, queued Resume) is a test; the "maybeContinue ignores dispose" mutant is red.
b2 / c2 Manager listeners outlived dispose(); a late removed paused the loop and wrote into a removed directory valid Fixed: dispose detaches the listeners start attached (off on the port). A test covers events after dispose (no change, no write, 0 listeners); the "not detached" mutant is red.
a / c surviving mutation control()'s persist was not pinned as tracked valid Pinned: dispose waits for a gated control persist; the untracked mutant is red.
b surviving mutation Timer cancellation was not asserted valid Pinned with fake timers: a held loop's poll is cancelled; the mutant is red.

Verification:

  • src/main/goalLoop: 110/110, with 0 ENOENT warnings.
  • b's three concurrent runs at 4145f1cf were already at 0 warnings.
  • npx tsc -b is clean.
  • The verification pass (a/b/c) is queued under the reviewer cap.

Round 2 (verification at 7a1d9c0d: a, b, c FIX-BEFORE-MERGE):

# Finding Verdict Disposition
a1 / b1 / c1 A delivery in flight when dispose() began retried and could schedule a backoff valid Fixed in 8fd8b3f5: the retry is skipped once disposed, and defer is inert after dispose. Pinned with a gated delivery plus fake timers.
b2 start() racing dispose() attached listeners and wrote valid Fixed: start returns after its read if disposal began. Pinned with a gated read.
a2 / b3 / c2 / a3 / c3 Public entry points wrote or scheduled after dispose() valid Fixed: startLoop, complete and carry reject; control returns null; hooks are ignored. Pinned.

Five mutations killed. 113/113, with 0 ENOENT warnings. Round 2 is the cap, so acceptance is the manager's call.

Juliusolsson05 and others added 2 commits September 27, 2026 01:12
…gins (review round 2)

Round 2 of #1371 (a, b, c):
- a delivery suspended when dispose() began resumed and retried (and
  could schedule a backoff): the retry is skipped once disposed, and
  defer() creates no timer after dispose; the in-flight outcome is still
  recorded;
- start() racing dispose() attached listeners and persisted afterwards:
  it now returns after its read if disposal began;
- public entry points after dispose: startLoop/complete/carry reject,
  control returns null, observeProviderHook is ignored.

Mutations killed: retry ignoring dispose, start ignoring dispose, defer
scheduling after dispose, public mutators accepting, control after
dispose. 113/113, 0 ENOENT warnings, tsc clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iew, b)

Round-1 b's surviving mutation (interrupt persisting fire-and-forget)
still passed 113/113; dispose now provably waits for a parked interrupt
write, and the mutant fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05
Juliusolsson05 merged commit ca858e2 into main Sep 27, 2026
2 checks passed
Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
@Juliusolsson05
Juliusolsson05 deleted the fix/goal-loop-drain branch September 27, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C9-tests Test hygiene sev:P3 Minor type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(goal-loop): GoalLoopService tests leave persistence running after cleanup (ENOENT noise)

1 participant