fix: remove stale waiter in waitForState timeout handler

[YunchuWang]

Fixes #201.

The bug

waitForState()'s timeout handler tried to remove its own waiter with
waiters.findIndex((w) => w.resolve === resolve). But w.resolve is a wrapper closure
((result) => { clearTimeout(timer); resolve(result); }), not the Promise's original
resolve. Two different function objects, so findIndex always returned -1 and the
splice was dead code.

Root cause is a circular dependency: the waiter needs timer (to clearTimeout), and the
timer callback needs waiter (to remove it). The original code declared timer first, so
its callback had no waiter in scope and fell back to comparing resolve — which never
matches.

Verified against unpatched main:

stateWaiters still has key      : true    (should be gone)
waiters array length            : 1       (should be 0)
after 5 timeouts, waiters       : 5       (unbounded accumulation)
stale predicate re-evaluated    : true    (notifyWaiters keeps running dead predicates)

No data corruption — resolving an already-rejected Promise is a no-op — but the waiter and
its closure leak, and notifyWaiters() keeps evaluating dead predicates until reset().

The fix

Declare waiter before the timer. The wrapper reads timer only when invoked, by which
point it has been assigned, so the circular dependency dissolves and the timeout callback can
use object identity:

• waiters.indexOf(waiter) instead of the broken findIndex reference comparison
• track the timeout timer in pendingTimers, and remove it on resolve / reject / timeout
• delete the stateWaiters map entry when its last waiter is removed

Validation

The two new tests fail on unpatched main:

Test	Unpatched main	With fix
should clean up stale waiters after waitForState timeout	FAIL — Expected: false, Received: true	pass
should remove only the timed-out waiter when multiple waiters exist	crashes the Node process	pass

The crash is a real consequence: the assertion fails, so the test's own reset() + .catch()
cleanup never runs, leaving an unhandled Backend was reset rejection.

With the fix: build 0, lint 0, 1178/1178 tests across 67 suites.

Note for reviewers — rebase and conflict resolution

This branch was rebased onto current main (it predated executionId on
OrchestrationInstance and no longer compiled against it).

The rebase conflicted with the timeoutMs === 0 "wait indefinitely" feature that landed on
main after this branch was cut. Taking either side wholesale would have been wrong — this
branch created the timer unconditionally, which with timeoutMs = 0 would fire a setTimeout(…, 0)
immediately and break that feature. The resolution keeps main's if (timeoutMs > 0) guard and
the timer: … | undefined type, and applies the fix inside it.

Both timeoutMs = 0 tests still pass:

√ waitForState with zero timeout should wait indefinitely until state matches
√ waitForState with zero timeout should be rejected on reset
√ should clean up stale waiters after waitForState timeout
√ should remove only the timed-out waiter when multiple waiters exist

One correction to #201

The issue lists a fourth impact:

Timer leak: The waitForState timeout timer is not tracked in pendingTimers, so it is not
cleaned up by reset().

That is not accurate. reset() iterates stateWaiters and calls waiter.reject(...), which
is the wrapper that already calls clearTimeout(timer):

clearTimeout calls on reset : 1
timer cleared by reset?     : true

Adding the timer to pendingTimers is defensive bookkeeping, not a leak fix. The other three
impacts in the issue are accurate.

[Copilot]

Pull request overview

Fixes a leak in the in-memory testing backend where waitForState() timeout handlers failed to remove stale waiters due to an identity/reference mismatch, causing stateWaiters to grow over time and timers to remain untracked across reset().

Changes:

• Fix waiter cleanup on timeout by removing the exact waiter object (indexOf(waiter)) rather than comparing function references.
• Track waitForState() timeout timers in pendingTimers, and remove them on resolve/reject/timeout so reset() can reliably clear them.
• Add regression tests to ensure timed-out waiters are removed (both single-waiter and multi-waiter scenarios).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/durabletask-js/src/testing/in-memory-backend.ts	Fixes waitForState() timeout cleanup and ensures timeout timers are tracked/removed via pendingTimers.
packages/durabletask-js/test/in-memory-backend.spec.ts	Adds regression tests verifying stale waiter cleanup after timeouts (single and multiple waiters).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

packages/durabletask-js/test/in-memory-backend.spec.ts:978

• This test depends on real 50ms/60s timers. Using real timers can be flaky and (if an assertion throws before cleanup) can leave a 60s timer keeping the Jest process alive longer than necessary. Using fake timers makes the timeout deterministic and avoids long-lived real timers.

    // Start a waiter with a long timeout (won't time out during the test)
    const longWaitPromise = backend.waitForState(
      instanceId,
      () => false, // Never matches
      60000, // 60 second timeout — won't fire
    );

    // Start a waiter with a very short timeout
    await expect(
      backend.waitForState(
        instanceId,
        () => false, // Never matches
        50, // 50ms timeout
      ),
    ).rejects.toThrow("Timeout waiting for orchestration");

packages/durabletask-js/test/in-memory-backend.spec.ts:948

• The test relies on a real 50ms timeout. This can be flaky under CI load and can intermittently fail if timers don’t fire promptly. Prefer Jest fake timers and advance the clock deterministically so the timeout handler is exercised reliably.

This issue also appears on line 963 of the same file.

    // Call waitForState with a predicate that will never match and a short timeout
    await expect(
      backend.waitForState(
        instanceId,
        () => false, // Will never match
        50, // 50ms timeout
      ),
    ).rejects.toThrow("Timeout waiting for orchestration");