test(event-log-delta): drain taskHistory in afterAll so history-contract test doesn't bleed #1046

Merged

reviewer merged 1 commit from fix/event-log-history-bleed into main

2026-05-10 14:34:16 +00:00

claude-desktop commented

2026-05-10 14:16:14 +00:00

Collaborator

Same class of bleed as PR #1032's pipeline-stall fix

Investigated PR #1042 + #1044 CI failures: all 4 failing tests are cross-file pollution from event-log-delta.test.ts. It calls getOrCreateRecord to seed records into event-log.ts's module-level taskHistory ring buffer, then never drains. When database/history-contract.test.ts runs after, it reads getTaskHistory() to assert the row shape exposed to the web app and sees foreign records that lack the contract-required keys.

Failing tests (every PR that hits this test ordering):

GET /history — web-app contract › each row only emits keys …
handleIssueUnassigned (#301) › running task → abort fired …
handleIssueUnassigned (#301) › queued tasks only → drain …
cancelStaleIssueTask › broadcasts dispatch.duplicate_running …

This is what kept #1042 / #1044 looping until #1045 landed.

Fix

Mirror the _resetConfigForTest pattern from webhook-config.ts:

Add __resetHistoryForTest() to event-log.ts — drains taskHistory.length = 0.
Call from event-log-delta.test.ts afterAll.

Verified

bun test src (server, full suite): 3449 / 3449 pass.
Reproduced the failing ordering before the fix; now green.

Why this matters beyond #1042

Same class of failure shape as pipeline-stall.test.ts ↔ completion-proof.test.ts. Pre-push test gate was disabled in dfe67c8b exactly because of these flaky orderings. Each fix narrows the window. Once enough singletons get drained, restoring the pre-push test step becomes safe again.

Refs: PR #1032 (precedent fix), PR #1042 / #1044 (incident).

## Same class of bleed as PR #1032's pipeline-stall fix Investigated PR #1042 + #1044 CI failures: all 4 failing tests are cross-file pollution from `event-log-delta.test.ts`. It calls `getOrCreateRecord` to seed records into `event-log.ts`'s module-level `taskHistory` ring buffer, then never drains. When `database/history-contract.test.ts` runs after, it reads `getTaskHistory()` to assert the row shape exposed to the web app and sees foreign records that lack the contract-required keys. Failing tests (every PR that hits this test ordering): ``` GET /history — web-app contract › each row only emits keys … handleIssueUnassigned (#301) › running task → abort fired … handleIssueUnassigned (#301) › queued tasks only → drain … cancelStaleIssueTask › broadcasts dispatch.duplicate_running … ``` This is what kept #1042 / #1044 looping until #1045 landed. ## Fix Mirror the `_resetConfigForTest` pattern from `webhook-config.ts`: - Add `__resetHistoryForTest()` to `event-log.ts` — drains `taskHistory.length = 0`. - Call from `event-log-delta.test.ts` `afterAll`. ## Verified - `bun test src` (server, full suite): 3449 / 3449 pass. - Reproduced the failing ordering before the fix; now green. ## Why this matters beyond #1042 Same class of failure shape as `pipeline-stall.test.ts` ↔ `completion-proof.test.ts`. Pre-push test gate was disabled in `dfe67c8b` exactly because of these flaky orderings. Each fix narrows the window. Once enough singletons get drained, restoring the pre-push test step becomes safe again. Refs: PR #1032 (precedent fix), PR #1042 / #1044 (incident).

claude-desktop added 1 commit

2026-05-10 14:16:14 +00:00

test(event-log-delta): drain taskHistory in afterAll so history-contract test doesn't bleed

qa / sql-layer-check (pull_request) Successful in 8s

Details

qa / i18n-string-check (pull_request) Successful in 13s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

qa / db-schema (pull_request) Successful in 15s

Details

qa / qa-1 (pull_request) Successful in 3m23s

Details

qa / qa (pull_request) Successful in 0s

Details

14df0ee098

Same class of bleed as the pipeline-stall fix in PR #1032: a test file
mutates a module-level singleton (`taskHistory` ring buffer in
`event-log.ts`) without restoring state. Sibling test
`database/history-contract.test.ts` reads `getTaskHistory()` to assert
the row shape exposed to the web app — when scheduled after
`event-log-delta.test.ts`, it sees the delta tests' inserted records
(which lack the contract-required keys) and fails 4 cross-file tests:

  GET /history — web-app contract › each row only emits keys …
  handleIssueUnassigned (#301) › running task → abort fired …
  handleIssueUnassigned (#301) › queued tasks only → drain …
  cancelStaleIssueTask › broadcasts dispatch.duplicate_running …

Failures hit PRs #1042 + #1044 (and looped fix-ci with no logs until
#1045 landed); #1043 will hit the same wall on rebase.

Add `__resetHistoryForTest()` to `event-log.ts`, mirror the pattern
from `_resetConfigForTest` in `webhook-config.ts`. Drain in
`event-log-delta.test.ts` `afterAll`. Full suite 3449/3449.

charles requested review from reviewer

2026-05-10 14:33:45 +00:00

reviewer approved these changes

2026-05-10 14:34:10 +00:00

reviewer left a comment

Correct fix. taskHistory.length = 0 is the right drain idiom; afterAll placement is correct; mirrors the _resetConfigForTest pattern cleanly. CI green.

Correct fix. `taskHistory.length = 0` is the right drain idiom; `afterAll` placement is correct; mirrors the `_resetConfigForTest` pattern cleanly. CI green.

reviewer merged commit fa03fc951a into main

2026-05-10 14:34:16 +00:00

charles referenced this pull request from a commit

2026-05-10 14:58:12 +00:00

test(agent-config-routes): drop process-global mock.module of ../sse