fix(test): abort leaked watch_action_run pollers between tests #23

Merged

ses merged 1 commit from fix/watch-leak-tests into main 2026-08-31 22:25:25 +00:00

Conversation 0 Commits 1 Files changed 2 +51 -4

ses commented 2026-08-31 22:23:33 +00:00

Owner

Copy link

Summary

• Abort leaked watch_action_run pollers between tests via a module-level watchers registry and a file-level afterEach in plugin.test.ts.
• Add resetAllWatchersForTests() to runtime.ts that aborts every Watchers instance's controllers; the existing signal.aborted check in watchActionRun.ts halts the loop with no further fetches.
• Add a guard test ("does not poll after the test ends (leak isolation)") that starts an always-"running" watcher, resets, swaps to a strict fetch mock, and asserts it is never called.
• Replace the no-op beforeEach(() => {}) with the real afterEach cleanup; drop the now-unused beforeEach import.

Why

• Symptom: 5 tests in plugin.test.ts (all located after the watch_action_run describe, all asserting on fetchMock.mock.calls[0]) failed intermittently and consistently under the full suite. Every failure showed the same signature — the "Received" URL was …/actions/runs/1 (the watcher's poll URL), not the failing test's own URL.
• Root cause: watch_action_run's execute is fire-and-forget (void runWatcher(...)). Two of the three watch tests never drive the run to a terminal status, and nothing aborted the resulting immortal setTimeout(0) poll loop when the test ended. Each loadPlugin makes a fresh createRuntime → fresh Watchers, so the leaked controller was unreachable from later tests. The leaked poller read globalThis.fetch live at call time (runtime.ts), so it landed in the next test's fetch mock and corrupted calls[0] / call counts.
• Chosen policy: a module registry + file-level afterEach. Watchers.reset() already aborts every controller; watchActionRun.ts checks signal.aborted before each fetch, so abort cleanly halts the loop. A file-level hook catches leaks from any test and needs no threading of runtime into afterEach scope. No change to the production plugin return contract ({ tool } unchanged).
• Rejected alternatives: (a) exposing runtime on the plugin return — changes the production return contract; (b) binding runtime.fetch at createRuntime creation — defense in depth but more invasive, deferred per YAGNI since abort already stops fetches; (c) asserting on the last matching call — treats the symptom, hides real bugs.

Verification

• bun run typecheck: clean. bun run lint (biome check): clean (92 files).
• Full suite 5x: 497 pass, 0 fail each (previously 5/5 runs failed, all 5 flaky tests).
• Watch-excluded run (--test-name-pattern '^(?!.*watch_action_run).*$'): 492 pass, 0 fail (baseline unaffected).
• New guard test passes in isolation.

Out of scope

• Binding runtime.fetch at creation time (report fix #3) — deferred per YAGNI; abort isolation is sufficient.

## Summary - Abort leaked `watch_action_run` pollers between tests via a module-level watchers registry and a file-level `afterEach` in `plugin.test.ts`. - Add `resetAllWatchersForTests()` to `runtime.ts` that aborts every `Watchers` instance's controllers; the existing `signal.aborted` check in `watchActionRun.ts` halts the loop with no further fetches. - Add a guard test ("does not poll after the test ends (leak isolation)") that starts an always-`"running"` watcher, resets, swaps to a strict fetch mock, and asserts it is never called. - Replace the no-op `beforeEach(() => {})` with the real `afterEach` cleanup; drop the now-unused `beforeEach` import. ## Why - Symptom: 5 tests in `plugin.test.ts` (all located after the `watch_action_run` describe, all asserting on `fetchMock.mock.calls[0]`) failed intermittently and consistently under the full suite. Every failure showed the same signature — the "Received" URL was `…/actions/runs/1` (the watcher's poll URL), not the failing test's own URL. - Root cause: `watch_action_run`'s `execute` is fire-and-forget (`void runWatcher(...)`). Two of the three watch tests never drive the run to a terminal status, and nothing aborted the resulting immortal `setTimeout(0)` poll loop when the test ended. Each `loadPlugin` makes a fresh `createRuntime` → fresh `Watchers`, so the leaked controller was unreachable from later tests. The leaked poller read `globalThis.fetch` live at call time (`runtime.ts`), so it landed in the next test's fetch mock and corrupted `calls[0]` / call counts. - Chosen policy: a module registry + file-level `afterEach`. `Watchers.reset()` already aborts every controller; `watchActionRun.ts` checks `signal.aborted` before each fetch, so abort cleanly halts the loop. A file-level hook catches leaks from any test and needs no threading of `runtime` into `afterEach` scope. No change to the production plugin return contract (`{ tool }` unchanged). - Rejected alternatives: (a) exposing `runtime` on the plugin return — changes the production return contract; (b) binding `runtime.fetch` at `createRuntime` creation — defense in depth but more invasive, deferred per YAGNI since abort already stops fetches; (c) asserting on the last matching call — treats the symptom, hides real bugs. ## Verification - `bun run typecheck`: clean. `bun run lint` (biome check): clean (92 files). - Full suite 5x: 497 pass, 0 fail each (previously 5/5 runs failed, all 5 flaky tests). - Watch-excluded run (`--test-name-pattern '^(?!.*watch_action_run).*$'`): 492 pass, 0 fail (baseline unaffected). - New guard test passes in isolation. ## Out of scope - Binding `runtime.fetch` at creation time (report fix #3) — deferred per YAGNI; abort isolation is sufficient.

ses added 1 commit 2026-08-31 22:23:33 +00:00

fix(test): abort leaked watch_action_run pollers between tests 4d1819115b

ses merged commit 4d1819115b into main 2026-08-31 22:25:25 +00:00

ses deleted branch fix/watch-leak-tests 2026-08-31 22:25:25 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

sebatec-eu/forgejoctl!23

No description provided.