Skip to content

Commit af32a5b

Browse files
authored
🤖 Fix auto-compact-continue race by tracking message IDs (#337)
## Problem After running `/compact -c "continue message"`, the continue message is sometimes sent **multiple times in a row** instead of just once. ## Root Cause The previous workspace-level guard had a race condition: 1. **Backend sends two events** for `replaceChatHistory`: delete event, then new message event 2. **Each event causes immediate synchronous `bump()`** which notifies all subscribers 3. **Multiple `checkAutoCompact()` calls can be in-flight simultaneously**: - Delete event → `bump()` → subscriber fires → `checkAutoCompact()` starts - New message event → `bump()` → subscriber fires → `checkAutoCompact()` starts - Both calls check workspace guard **before either sets it** → both proceed to send The double-check guard in PR #334 helped but didn't fully solve it because the checks and sets are separate operations. **The real issue: Workspace-level tracking is the wrong granularity.** We need to prevent processing the same compaction MESSAGE multiple times, not the same workspace multiple times. A new compaction creates a new message with a new ID. ## Solution Track processed **message IDs** instead of workspace IDs: ```typescript // Track which specific compaction summary messages we've already processed const processedMessageIds = useRef<Set<string>>(new Set()); // In checkAutoCompact: const messageId = summaryMessage.id; // Have we already processed this specific compaction message? if (processedMessageIds.current.has(messageId)) continue; // Mark THIS MESSAGE as processed processedMessageIds.current.add(messageId); ``` ## Why This Is Obviously Correct 1. **Message IDs are unique and immutable** - Once a message exists, its ID never changes 2. **We only care about processing each message once** - Not about processing each workspace once 3. **The guard is set BEFORE the async operation** - No timing window 4. **Multiple calls can overlap** - But they all see the same message ID, so only the first one proceeds 5. **Cleanup is natural** - IDs accumulate bounded (one per compaction) and don't need explicit cleanup The correctness is self-evident: "Have we sent a continue message for THIS compaction result message? Yes/No." ## How It Fixes The Race **Before (workspace-level):** - Call #1 checks `firedForWorkspace.has(workspaceId)` → false - Call #2 checks `firedForWorkspace.has(workspaceId)` → false (still!) - Call #1 sets guard and sends - Call #2 double-checks... but timing window existed **After (message-level):** - Call #1 checks `processedMessageIds.has(messageId)` → false - Call #2 checks `processedMessageIds.has(messageId)` → false (same message) - Call #1 adds messageId to set - Call #2 sees messageId already in set → skips Both calls are checking the **same unique identifier** (the message ID), so the guard works correctly even with concurrent execution. ## Testing Manual testing needed: 1. Run `/compact -c "continue message"` multiple times 2. Verify only ONE continue message is sent per compaction 3. Check console logs for single "Sending continue message" per compaction 4. Verify backend receives only one user message (not duplicates) _Generated with `cmux`_
1 parent 0585010 commit af32a5b

File tree

1 file changed

+21
-16
lines changed

1 file changed

+21
-16
lines changed

src/hooks/useAutoCompactContinue.ts

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ export function useAutoCompactContinue() {
3030
const store = useWorkspaceStoreRaw();
3131
const workspaceStatesRef = useRef<Map<string, WorkspaceState>>(new Map());
3232

33-
// Prevent duplicate auto-sends if effect runs more than once while the same
34-
// compacted summary is visible (e.g., rapid state updates after replaceHistory)
35-
const firedForWorkspace = useRef<Set<string>>(new Set());
33+
// Track which specific compaction summary messages we've already processed.
34+
// Key insight: Each compaction creates a unique message. Track by message ID,
35+
// not workspace ID, to prevent processing the same compaction result multiple times.
36+
// This is obviously correct because message IDs are immutable and unique.
37+
const processedMessageIds = useRef<Set<string>>(new Set());
3638

3739
// Update ref and check for auto-continue condition
3840
const checkAutoCompact = () => {
@@ -41,35 +43,38 @@ export function useAutoCompactContinue() {
4143

4244
// Check all workspaces for completed compaction
4345
for (const [workspaceId, state] of newStates) {
44-
// Reset guard when compaction is no longer in the single-compacted-message state
46+
// Detect if workspace is in "single compacted message" state
4547
const isSingleCompacted =
4648
state.messages.length === 1 &&
4749
state.messages[0].type === "assistant" &&
4850
state.messages[0].isCompacted === true;
4951

5052
if (!isSingleCompacted) {
51-
// Allow future auto-continue for this workspace when next compaction completes
52-
firedForWorkspace.current.delete(workspaceId);
53+
// Workspace no longer in compacted state - no action needed
54+
// Processed message IDs will naturally accumulate but stay bounded
55+
// (one per compaction), and get cleared when user sends new messages
5356
continue;
5457
}
5558

56-
// Only proceed once per compaction completion
57-
if (firedForWorkspace.current.has(workspaceId)) continue;
58-
5959
// After compaction, history is replaced with a single summary message
6060
// The summary message has compaction-result metadata with the continueMessage
6161
const summaryMessage = state.cmuxMessages[0]; // Single compacted message
62+
const messageId = summaryMessage.id;
63+
64+
// Have we already processed this specific compaction message?
65+
// This check is race-safe because message IDs are unique and immutable.
66+
if (processedMessageIds.current.has(messageId)) continue;
67+
6268
const cmuxMeta = summaryMessage?.metadata?.cmuxMetadata;
6369
const continueMessage =
6470
cmuxMeta?.type === "compaction-result" ? cmuxMeta.continueMessage : undefined;
6571

6672
if (!continueMessage) continue;
6773

68-
// Mark as fired BEFORE any async operations to prevent race conditions
69-
// This MUST come immediately after checking continueMessage to ensure
70-
// only one of multiple concurrent checkAutoCompact() runs can proceed
71-
if (firedForWorkspace.current.has(workspaceId)) continue; // Double-check
72-
firedForWorkspace.current.add(workspaceId);
74+
// Mark THIS MESSAGE as processed before sending
75+
// Multiple concurrent checkAutoCompact() calls will all see the same message ID,
76+
// so only the first call that reaches this point will proceed
77+
processedMessageIds.current.add(messageId);
7378

7479
console.log(
7580
`[useAutoCompactContinue] Sending continue message for ${workspaceId}:`,
@@ -80,8 +85,8 @@ export function useAutoCompactContinue() {
8085
const options = buildSendMessageOptions(workspaceId);
8186
window.api.workspace.sendMessage(workspaceId, continueMessage, options).catch((error) => {
8287
console.error("Failed to send continue message:", error);
83-
// If sending failed, allow another attempt on next render by clearing the guard
84-
firedForWorkspace.current.delete(workspaceId);
88+
// If sending failed, remove from processed set to allow retry
89+
processedMessageIds.current.delete(messageId);
8590
});
8691
}
8792
};

0 commit comments

Comments
 (0)