fix: ensure anthropic-auto-compact lock is always cleared (#232)

Fixes #200

## Problem
When executeCompact() recovery fails unexpectedly or gets interrupted,
the compactionInProgress lock is never cleared, permanently blocking both
auto-compact AND manual /compact for the session.

## Root Cause
- No try/finally around lock acquisition (line 261)
- Silent blocking when lock held - no user feedback
- Lock cleanup scattered across 7 manual deletion points
- Any unexpected exception bypasses cleanup, leaving lock stuck forever

## Solution
1. **Try/Finally Lock Guarantee**: Wrapped entire executeCompact body in
   try/finally block to guarantee lock cleanup, following the pattern
   used in preemptive-compaction hook

2. **User Feedback**: Added toast notification when compact attempt is
   blocked by existing lock, replacing silent failure with clear warning

3. **Removed Redundancy**: Removed 6 redundant manual lock deletions
   (kept only clearSessionState and finally block)

## Testing Evidence
 10/10 comprehensive tests pass
 Lock cleared on successful completion
 Lock cleared when summarize throws
 Lock cleared when revert throws
 Lock cleared when fixEmptyMessages executes
 Lock cleared when truncation is sufficient
 Lock cleared after max recovery attempts
 Lock cleared when toast fails
 Lock cleared when prompt_async throws
 Toast shown when lock already held
 TypeScript type check passes with zero errors

## Files Changed
- executor.ts: Added try/finally, toast notification, removed 6 redundant deletions
- executor.test.ts: New comprehensive test suite (10 tests, 13 assertions)

## Impact
- Severity: High → Fixed
- User Experience: No more stuck sessions requiring restart
- Behavior: Identical except lock now guaranteed to clear

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
This commit is contained in:
Sisyphus
2025-12-25 19:36:22 +09:00
committed by GitHub
parent 3b17ee9bd0
commit 06b77643ba
2 changed files with 731 additions and 395 deletions

View File

@@ -0,0 +1,260 @@
import { describe, test, expect, mock, beforeEach } from "bun:test"
import { executeCompact } from "./executor"
import type { AutoCompactState } from "./types"
describe("executeCompact lock management", () => {
let autoCompactState: AutoCompactState
let mockClient: any
const sessionID = "test-session-123"
const directory = "/test/dir"
const msg = { providerID: "anthropic", modelID: "claude-opus-4-5" }
beforeEach(() => {
// #given: Fresh state for each test
autoCompactState = {
pendingCompact: new Set<string>(),
errorDataBySession: new Map(),
retryStateBySession: new Map(),
fallbackStateBySession: new Map(),
truncateStateBySession: new Map(),
emptyContentAttemptBySession: new Map(),
compactionInProgress: new Set<string>(),
}
mockClient = {
session: {
messages: mock(() => Promise.resolve({ data: [] })),
summarize: mock(() => Promise.resolve()),
revert: mock(() => Promise.resolve()),
prompt_async: mock(() => Promise.resolve()),
},
tui: {
showToast: mock(() => Promise.resolve()),
},
}
})
test("clears lock on successful summarize completion", async () => {
// #given: Valid session with providerID/modelID
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction successfully
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Lock should be cleared
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("clears lock when summarize throws exception", async () => {
// #given: Summarize will fail
mockClient.session.summarize = mock(() =>
Promise.reject(new Error("Network timeout")),
)
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Lock should still be cleared despite exception
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("clears lock when revert throws exception", async () => {
// #given: Force revert path by exhausting retry attempts and making revert fail
mockClient.session.revert = mock(() =>
Promise.reject(new Error("Revert failed")),
)
mockClient.session.messages = mock(() =>
Promise.resolve({
data: [
{ info: { id: "msg1", role: "user" } },
{ info: { id: "msg2", role: "assistant" } },
],
}),
)
// Exhaust retry attempts
autoCompactState.retryStateBySession.set(sessionID, {
attempt: 5,
lastAttemptTime: Date.now(),
})
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Lock cleared even though revert failed
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("shows toast when lock already held", async () => {
// #given: Lock already held
autoCompactState.compactionInProgress.add(sessionID)
// #when: Try to execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Toast should be shown with warning message
expect(mockClient.tui.showToast).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
title: "Compact In Progress",
message: expect.stringContaining("Recovery already running"),
variant: "warning",
}),
}),
)
// #then: compactionInProgress should still have the lock
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(true)
})
test("clears lock when fixEmptyMessages path executes", async () => {
// #given: Empty content error scenario
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "non-empty content required",
messageIndex: 0,
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction (fixEmptyMessages will be called)
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Lock should be cleared
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("clears lock when truncation is sufficient", async () => {
// #given: Aggressive truncation scenario with sufficient truncation
// This test verifies the early return path in aggressive truncation
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 250000,
maxTokens: 200000,
})
const experimental = {
truncate_all_tool_outputs: false,
aggressive_truncation: true,
}
// #when: Execute compaction with experimental flag
await executeCompact(
sessionID,
msg,
autoCompactState,
mockClient,
directory,
experimental,
)
// #then: Lock should be cleared even on early return
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("prevents concurrent compaction attempts", async () => {
// #given: Lock already held (simpler test)
autoCompactState.compactionInProgress.add(sessionID)
// #when: Try to execute compaction while lock is held
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Toast should be shown
const toastCalls = (mockClient.tui.showToast as any).mock.calls
const blockedToast = toastCalls.find(
(call: any) => call[0]?.body?.title === "Compact In Progress",
)
expect(blockedToast).toBeDefined()
// #then: Lock should still be held (not cleared by blocked attempt)
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(true)
})
test("clears lock after max recovery attempts exhausted", async () => {
// #given: All retry/revert attempts exhausted
mockClient.session.messages = mock(() => Promise.resolve({ data: [] }))
// Max out all attempts
autoCompactState.retryStateBySession.set(sessionID, {
attempt: 5,
lastAttemptTime: Date.now(),
})
autoCompactState.fallbackStateBySession.set(sessionID, {
revertAttempt: 5,
})
autoCompactState.truncateStateBySession.set(sessionID, {
truncateAttempt: 5,
})
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Should show failure toast
const toastCalls = (mockClient.tui.showToast as any).mock.calls
const failureToast = toastCalls.find(
(call: any) => call[0]?.body?.title === "Auto Compact Failed",
)
expect(failureToast).toBeDefined()
// #then: Lock should still be cleared
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("clears lock when client.tui.showToast throws", async () => {
// #given: Toast will fail (this should never happen but testing robustness)
mockClient.tui.showToast = mock(() =>
Promise.reject(new Error("Toast failed")),
)
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// #then: Lock should be cleared even if toast fails
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
test("clears lock when prompt_async in continuation throws", async () => {
// #given: prompt_async will fail during continuation
mockClient.session.prompt_async = mock(() =>
Promise.reject(new Error("Prompt failed")),
)
autoCompactState.errorDataBySession.set(sessionID, {
errorType: "token_limit",
currentTokens: 100000,
maxTokens: 200000,
})
// #when: Execute compaction
await executeCompact(sessionID, msg, autoCompactState, mockClient, directory)
// Wait for setTimeout callback
await new Promise((resolve) => setTimeout(resolve, 600))
// #then: Lock should be cleared
// The continuation happens in setTimeout, but lock is cleared in finally before that
expect(autoCompactState.compactionInProgress.has(sessionID)).toBe(false)
})
})

File diff suppressed because it is too large Load Diff