test(acp): add OOM fix behavior coverage (#4052)#4053
Merged
Conversation
Follow-up to PR #4051 (OOM crash loop fix). Adds 11 tests covering: 1. Event compaction enforcement β verifies maxEventsPerSession pruning 2. Startup pruning of terminal sessions β closed/failed events removed on load 3. Debounce coalescing β multiple rapid mutations β single disk write 4. Flush lifecycle β dirty state persisted on stop(), health checks 5. Incremental seq tracking β Map rebuild on load, per-session independence 6. Lightweight serialization β complex payload round-trip, valid JSON output All 6 behaviors from the issue acceptance criteria covered. Gate: tsc β, build β, 5072/5080 tests pass (8 skipped pre-existing).
Contributor
There was a problem hiding this comment.
Review: PR #4053 β OOM Fix Test Coverage (#4052)
Verdict: β Approved
All 6 OOM-prevention behaviors covered with 11 new tests. Test-only change, single file, clean isolation with scratch directories and finally cleanup.
Coverage Map
| Behavior | Tests | Verdict |
|---|---|---|
| Event compaction enforcement | 2 | β Oldest pruned, per-session independent |
| Startup pruning of terminal sessions | 1 | β closed/failed pruned, running kept |
| Debounce coalescing | 1 | β 3 rapid mutations β single persist |
| Flush lifecycle | 2 | β stop() flushes dirty state + health checks |
| Incremental seq tracking | 2 | β Map rebuild on load, per-session independence |
| Lightweight serialization | 2 | β Nested payload round-trip + valid JSON output |
9 Merge Gates
- β Review completed
- β No conflicts β MERGEABLE
- β CI green (Node 22 pass, Node 20 pending β same suite)
- β No regressions β test-only change
- β Unit tests β this IS the tests (+11 new)
- β E2E β dashboard-e2e pass
- β Documented β PR body maps tests to behaviors
- β Security clean β all checks pass
- β
Targets
develop
Minor Nits (non-blocking)
- Line 253:
createMemoryAcpLocalStorageProfile()instantiated but never used β dead code completedstatus in PRUNABLE_SESSION_STATUSES is never tested (bothcompleted-1andclosed-1usestatus: 'closed') β minor gap- 500ms setTimeout in debounce test is timing-dependent β acceptable for integration test
None are blockers. Solid test coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #4051 (OOM crash loop fix). Adds comprehensive test coverage for the 6 OOM-prevention behaviors introduced in that fix.
Closes #4052
Tests Added (11 new)
1. Event compaction enforcement (2 tests)
maxEventsPerSession2. Startup pruning of terminal sessions (1 test)
closed/failedsessions onstart(), keepsrunningsession events3. Debounce coalescing (1 test)
4. Flush lifecycle (2 tests)
stop()flushes dirty state despite long debounce (fire-and-forget mutation pattern)5. Incremental seq tracking (2 tests)
lastEventSeqBySessionmap rebuilt correctly onloadState()from disk6. Lightweight serialization (2 tests)
Verification