test(continuation): pin request_compaction pending Set cleanup on failure + restart#465
Merged
silas-dandelion-cult merged 1 commit intocael/325-canonical2from May 1, 2026
Conversation
…lure + restart Adds [#447] two new tests to src/agents/tools/request-compaction-tool.test.ts: 1. clears pending compaction session after background rejection Pins request-compaction-tool.ts:233-235 (the .finally cleanup in the fire-and-forget chain). Without the .finally cleanup, a rejected triggerCompaction() would orphan the sessionKey in pendingCompactionSessions forever; every subsequent call would return already_pending until process restart. Trap shape: assert second call is NOT already_pending (rate-limit is the expected new gate; dedup-block is the bug we trap). Deliberately does NOT call _resetGuardState before the second call, since that helper clears the pending Set itself and would mask the bug. 2. _resetGuardState clears pending compaction sessions for restart/test isolation Pins request-compaction-tool.ts:273-280 (both branches: keyed and unkeyed). Test-only helper that simulates process restart for in-flight dedup. Covers both single-session reset and full-clear (with cross-session cleanup verified). Verified load-bearing on each test independently: - Test 1: removed .finally body in production -> test 1 fails with "expected { status: 'already_pending' } to not match object { status: 'already_pending' }". Restored. - Test 2: removed both .delete/.clear calls from _resetGuardState -> test 2 fails with "compaction_requested vs already_pending" diff. Restored. 24/24 passing on canonical2 cf7830f baseline. Closes #447
💡 Codex Reviewopenclaw/src/agents/tools/request-compaction-tool.ts Lines 205 to 206 in 4d2a08c
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
cael-dandelion-cult
approved these changes
May 1, 2026
cael-dandelion-cult
left a comment
There was a problem hiding this comment.
🩸 LGTM — test-only addition pinning .finally cleanup of pendingCompactionSessions on background rejection + _resetGuardState restart-isolation contract. Sabotage walks documented inline. Targets cael/325-canonical2.
afe015c
into
cael/325-canonical2
4 of 12 checks passed
ronan-dandelion-cult
pushed a commit
that referenced
this pull request
May 3, 2026
…lure + restart (#465) Adds [#447] two new tests to src/agents/tools/request-compaction-tool.test.ts: 1. clears pending compaction session after background rejection Pins request-compaction-tool.ts:233-235 (the .finally cleanup in the fire-and-forget chain). Without the .finally cleanup, a rejected triggerCompaction() would orphan the sessionKey in pendingCompactionSessions forever; every subsequent call would return already_pending until process restart. Trap shape: assert second call is NOT already_pending (rate-limit is the expected new gate; dedup-block is the bug we trap). Deliberately does NOT call _resetGuardState before the second call, since that helper clears the pending Set itself and would mask the bug. 2. _resetGuardState clears pending compaction sessions for restart/test isolation Pins request-compaction-tool.ts:273-280 (both branches: keyed and unkeyed). Test-only helper that simulates process restart for in-flight dedup. Covers both single-session reset and full-clear (with cross-session cleanup verified). Verified load-bearing on each test independently: - Test 1: removed .finally body in production -> test 1 fails with "expected { status: 'already_pending' } to not match object { status: 'already_pending' }". Restored. - Test 2: removed both .delete/.clear calls from _resetGuardState -> test 2 fails with "compaction_requested vs already_pending" diff. Restored. 24/24 passing on canonical2 cf7830f baseline. Closes #447
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
2 participants
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.
Closes #447.
What this PR adds
Two new tests in
src/agents/tools/request-compaction-tool.test.tscovering the two cleanup paths the existing suite leaves untrapped: background rejection and restart/test-isolation.Test 1:
clears pending compaction session after background rejectionThe existing suite covers
already_pendingand dedup-cleared-after-resolve (happy path). It does NOT cover the rejection path. Without the.finallycleanup atrequest-compaction-tool.ts:233-235, a rejectedtriggerCompaction()would orphan the sessionKey inpendingCompactionSessionsforever — every subsequent call would returnalready_pendinguntil process restart.Trap shape: assert second call is NOT
already_pending. Rate-limit (Guard 2) is the expected gate after a successful first call; dedup-block (Guard 0) is the bug we trap. Deliberately does NOT call_resetGuardStatebefore the second assertion — that helper clears the pending Set itself and would mask the bug.Test 2:
_resetGuardState clears pending compaction sessions for restart/test isolationDocuments and pins the test-only helper's restart contract. Covers both branches (
request-compaction-tool.ts:273-280):_resetGuardState(SESSION_KEY)clears pending for that session_resetGuardState()clears pending for ALL sessions (full restart simulation; verified by cross-session check)A refactor that splits the guard caches without updating
_resetGuardStatewould leave the dedup Set polluted across test boundaries — symptom would be flakyalready_pendingresults bleeding between tests.Verified load-bearing (independently)
Test 1 sabotage: removed
.finallybody in production:Test 2 sabotage: removed both
.delete/.clearcalls from_resetGuardState:Restored. 24/24 passing on canonical2
cf7830ffb3702bf7d826d70838893e2e41709f12baseline.Receipts
Refs
src/agents/tools/)cf7830ffb3702bf7d826d70838893e2e41709f12🌫️