Skip to content

Commit 3ffbb82

Browse files
ammar-agentammar-agent
authored
🤖 fix: suppress browser notifications for idle compaction completions (#2554)
## Summary Suppress browser notification noise for compaction flows by moving idle-compaction detection to activity snapshots, skipping compaction notifications when an automatic follow-up message is queued, and hardening idle-marker cleanup across race/error paths. ## Background The previous idle-compaction suppression relied on an `idle-compaction-started` chat event. That signal is unreliable for true background work because `onChat` is only active for the currently selected workspace. During Codex review, additional edge cases surfaced where stale idle markers could leak across turns and incorrectly suppress non-idle notifications. ## Implementation 1. **Activity snapshot contract** - Added optional `isIdleCompaction` to `WorkspaceActivitySnapshotSchema`. 2. **Backend idle marker lifecycle (`WorkspaceService`)** - Track in-flight idle maintenance with `idleCompactingWorkspaces`. - Set marker only after idle dispatch succeeds **and** the session is still busy. - Tag `isIdleCompaction` only on `streaming=false` snapshots (stop snapshots), not on `streaming=true` snapshots. - Always clear marker for `streaming=false` transitions in `finally`, so metadata write failures cannot leak state into future turns. 3. **Frontend completion suppression (`WorkspaceStore` + `App`)** - Use activity transitions (`streaming true→false` + recency advance) for background completion detection. - Preserve idle detection via `previous || snapshot` for reconnect/restore resilience. - Route completion callbacks through a store-level wrapper. - For compaction completions, if a queued follow-up exists (auto-sent next turn), coerce `hasContinueMessage=true` so App suppresses intermediate "Compaction complete" notifications. - `App.tsx` suppresses notifications for `compaction?.isIdle` and `compaction?.hasContinueMessage`. 4. **Cleanup** - Removed dead `idle-compaction-started` plumbing across backend/frontend: - backend emitter method - frontend callback subscription/export - stream schema branch and ignored-event handling 5. **Tests** - Updated idle-compaction dispatch tests. - Added regression coverage for: - no stale marker when send succeeds without active stream - marker cleared when `setStreaming` fails on stream-stop - `streaming=true` snapshots are never idle-tagged - active compaction completion with queued follow-up suppression path ## Validation - `make static-check` - `bun test src/browser/stores/WorkspaceStore.test.ts` - `bun test src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.ts` - `bun test src/node/services/workspaceService.test.ts` - `make typecheck` ## Risks Low risk and scoped to compaction completion notification behavior. Main risk is over-suppression in edge cases where queued follow-up intent is ambiguous, but this is preferable to duplicate/intermediate notifications and matches compaction auto-send semantics. ## Pains This needed several Codex-driven hardening passes for subtle ordering/failure edges (startup-abort, metadata write failure, and cross-turn marker bleed), which required tightening marker lifecycle guarantees rather than adding timing-based coordination. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$10.37`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=10.37 -->
1 parent 7ccc1d9 commit 3ffbb82

9 files changed

Lines changed: 387 additions & 99 deletions

File tree

‎src/browser/App.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ function AppInner() {
867867
_messageId: string,
868868
isFinal: boolean,
869869
finalText: string,
870-
compaction?: { hasContinueMessage: boolean },
870+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
871871
completedAt?: number | null
872872
) => {
873873
// Only notify on final message (when assistant is done with all work)
@@ -882,6 +882,9 @@ function AppInner() {
882882
updatePersistedState(getWorkspaceLastReadKey(workspaceId), completedAt);
883883
}
884884

885+
// Skip notification for idle compaction (background maintenance, not user-initiated).
886+
if (compaction?.isIdle) return;
887+
885888
// Skip notification if compaction completed with a continue message.
886889
// We use the compaction metadata instead of queued state since the queue
887890
// can be drained before compaction finishes.

‎src/browser/stores/WorkspaceStore.test.ts‎

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ describe("WorkspaceStore", () => {
11691169
_messageId: string,
11701170
_isFinal: boolean,
11711171
_finalText: string,
1172-
_compaction?: { hasContinueMessage: boolean },
1172+
_compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
11731173
_completedAt?: number | null
11741174
) => undefined
11751175
);
@@ -1297,7 +1297,7 @@ describe("WorkspaceStore", () => {
12971297
_messageId: string,
12981298
_isFinal: boolean,
12991299
_finalText: string,
1300-
_compaction?: { hasContinueMessage: boolean },
1300+
_compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
13011301
_completedAt?: number | null
13021302
) => undefined
13031303
);
@@ -1344,6 +1344,115 @@ describe("WorkspaceStore", () => {
13441344
);
13451345
});
13461346

1347+
it("marks compaction completions with queued follow-up as continue for active callbacks", async () => {
1348+
const workspaceId = "active-workspace-queued-follow-up";
1349+
1350+
mockOnChat.mockImplementation(async function* (
1351+
input?: { workspaceId: string; mode?: unknown },
1352+
options?: { signal?: AbortSignal }
1353+
): AsyncGenerator<WorkspaceChatMessage, void, unknown> {
1354+
if (input?.workspaceId !== workspaceId) {
1355+
await waitForAbortSignal(options?.signal);
1356+
return;
1357+
}
1358+
1359+
const timestamp = Date.now();
1360+
1361+
yield { type: "caught-up", hasOlderHistory: false };
1362+
1363+
yield {
1364+
type: "message",
1365+
id: "compaction-request-msg",
1366+
role: "user",
1367+
parts: [{ type: "text", text: "/compact" }],
1368+
metadata: {
1369+
historySequence: 1,
1370+
timestamp,
1371+
muxMetadata: {
1372+
type: "compaction-request",
1373+
rawCommand: "/compact",
1374+
parsed: {
1375+
model: "claude-sonnet-4",
1376+
},
1377+
},
1378+
},
1379+
};
1380+
1381+
yield {
1382+
type: "stream-start",
1383+
workspaceId,
1384+
messageId: "compaction-stream",
1385+
historySequence: 2,
1386+
model: "claude-sonnet-4",
1387+
startTime: timestamp + 1,
1388+
mode: "compact",
1389+
};
1390+
1391+
// A queued message will be auto-sent by the backend when compaction stream ends.
1392+
yield {
1393+
type: "queued-message-changed",
1394+
workspaceId,
1395+
queuedMessages: ["follow-up after compaction"],
1396+
displayText: "follow-up after compaction",
1397+
};
1398+
1399+
yield {
1400+
type: "stream-end",
1401+
workspaceId,
1402+
messageId: "compaction-stream",
1403+
metadata: {
1404+
model: "claude-sonnet-4",
1405+
},
1406+
parts: [],
1407+
};
1408+
1409+
await waitForAbortSignal(options?.signal);
1410+
});
1411+
1412+
const onResponseComplete = mock(
1413+
(
1414+
_workspaceId: string,
1415+
_messageId: string,
1416+
_isFinal: boolean,
1417+
_finalText: string,
1418+
_compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
1419+
_completedAt?: number | null
1420+
) => undefined
1421+
);
1422+
1423+
store.dispose();
1424+
store = new WorkspaceStore(mockOnModelUsed);
1425+
store.setOnResponseComplete(onResponseComplete);
1426+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-explicit-any
1427+
store.setClient(mockClient as any);
1428+
1429+
createAndAddWorkspace(store, workspaceId);
1430+
1431+
const waitUntil = async (condition: () => boolean, timeoutMs = 2000): Promise<boolean> => {
1432+
const start = Date.now();
1433+
while (Date.now() - start < timeoutMs) {
1434+
if (condition()) {
1435+
return true;
1436+
}
1437+
await new Promise((resolve) => setTimeout(resolve, 10));
1438+
}
1439+
return false;
1440+
};
1441+
1442+
const sawResponseComplete = await waitUntil(() => onResponseComplete.mock.calls.length > 0);
1443+
expect(sawResponseComplete).toBe(true);
1444+
1445+
expect(onResponseComplete).toHaveBeenCalledTimes(1);
1446+
expect(onResponseComplete).toHaveBeenCalledWith(
1447+
workspaceId,
1448+
"compaction-stream",
1449+
true,
1450+
"",
1451+
{ hasContinueMessage: true },
1452+
expect.any(Number)
1453+
);
1454+
});
1455+
13471456
it("does not fire response-complete callback when background streaming stops without recency advance", async () => {
13481457
const activeWorkspaceId = "active-workspace-no-replay";
13491458
const backgroundWorkspaceId = "background-workspace-no-replay";
@@ -1397,7 +1506,7 @@ describe("WorkspaceStore", () => {
13971506
_messageId: string,
13981507
_isFinal: boolean,
13991508
_finalText: string,
1400-
_compaction?: { hasContinueMessage: boolean },
1509+
_compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
14011510
_completedAt?: number | null
14021511
) => undefined
14031512
);

‎src/browser/stores/WorkspaceStore.ts‎

Lines changed: 96 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,6 @@ export class WorkspaceStore {
569569
private sessionUsage = new Map<string, z.infer<typeof SessionUsageFileSchema>>();
570570
private sessionUsageRequestVersion = new Map<string, number>();
571571

572-
// Idle compaction notification callbacks (called when backend signals idle compaction started)
573-
private idleCompactionCallbacks = new Set<(workspaceId: string) => void>();
574-
575572
// Global callback for navigating to a workspace (set by App, used for notification clicks)
576573
private navigateToWorkspaceCallback: ((workspaceId: string) => void) | null = null;
577574

@@ -585,7 +582,7 @@ export class WorkspaceStore {
585582
messageId: string,
586583
isFinal: boolean,
587584
finalText: string,
588-
compaction?: { hasContinueMessage: boolean },
585+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
589586
completedAt?: number | null
590587
) => void)
591588
| null = null;
@@ -1290,15 +1287,84 @@ export class WorkspaceStore {
12901287
messageId: string,
12911288
isFinal: boolean,
12921289
finalText: string,
1293-
compaction?: { hasContinueMessage: boolean },
1290+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
12941291
completedAt?: number | null
12951292
) => void
12961293
): void {
12971294
this.responseCompleteCallback = callback;
12981295
// Update existing aggregators with the callback
12991296
for (const aggregator of this.aggregators.values()) {
1300-
aggregator.onResponseComplete = callback;
1297+
this.bindAggregatorResponseCompleteCallback(aggregator);
1298+
}
1299+
}
1300+
1301+
private maybeMarkCompactionContinueFromQueuedFollowUp(
1302+
workspaceId: string,
1303+
compaction: { hasContinueMessage: boolean; isIdle?: boolean } | undefined,
1304+
includeQueuedFollowUpSignal: boolean
1305+
): { hasContinueMessage: boolean; isIdle?: boolean } | undefined {
1306+
if (!compaction || compaction.hasContinueMessage || !includeQueuedFollowUpSignal) {
1307+
return compaction;
1308+
}
1309+
1310+
const queuedMessage = this.chatTransientState.get(workspaceId)?.queuedMessage;
1311+
if (!queuedMessage) {
1312+
return compaction;
13011313
}
1314+
1315+
// A queued message will be auto-sent after stream-end. Suppress the intermediate
1316+
// "Compaction complete" notification and only notify for the follow-up response.
1317+
return {
1318+
...compaction,
1319+
hasContinueMessage: true,
1320+
};
1321+
}
1322+
1323+
private emitResponseComplete(
1324+
workspaceId: string,
1325+
messageId: string,
1326+
isFinal: boolean,
1327+
finalText: string,
1328+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
1329+
completedAt?: number | null,
1330+
includeQueuedFollowUpSignal = true
1331+
): void {
1332+
if (!this.responseCompleteCallback) {
1333+
return;
1334+
}
1335+
1336+
this.responseCompleteCallback(
1337+
workspaceId,
1338+
messageId,
1339+
isFinal,
1340+
finalText,
1341+
this.maybeMarkCompactionContinueFromQueuedFollowUp(
1342+
workspaceId,
1343+
compaction,
1344+
includeQueuedFollowUpSignal
1345+
),
1346+
completedAt
1347+
);
1348+
}
1349+
1350+
private bindAggregatorResponseCompleteCallback(aggregator: StreamingMessageAggregator): void {
1351+
aggregator.onResponseComplete = (
1352+
workspaceId: string,
1353+
messageId: string,
1354+
isFinal: boolean,
1355+
finalText: string,
1356+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
1357+
completedAt?: number | null
1358+
) => {
1359+
this.emitResponseComplete(
1360+
workspaceId,
1361+
messageId,
1362+
isFinal,
1363+
finalText,
1364+
compaction,
1365+
completedAt
1366+
);
1367+
};
13021368
}
13031369

13041370
/**
@@ -2369,25 +2435,35 @@ export class WorkspaceStore {
23692435
const backgroundCompaction = isBackgroundStreamingStop
23702436
? this.getBackgroundCompletionCompaction(workspaceId)
23712437
: undefined;
2438+
// The backend tags the streaming=false (stop) snapshot with isIdleCompaction.
2439+
// The idle marker is added after sendMessage returns (to avoid races with
2440+
// concurrent user streams), so only the stop snapshot carries the flag.
2441+
// Check both previous and current as defense-in-depth.
2442+
const wasIdleCompaction =
2443+
previous?.isIdleCompaction === true || snapshot?.isIdleCompaction === true;
23722444

23732445
// Trigger response completion notifications for background workspaces only when
23742446
// activity indicates a true completion (streaming true -> false WITH recency advance).
23752447
// stream-abort/error transitions also flip streaming to false, but recency stays
23762448
// unchanged there, so suppress completion notifications in those cases.
23772449
if (stoppedStreamingSnapshot && recencyAdvancedSinceStreamStart && isBackgroundStreamingStop) {
2378-
if (this.responseCompleteCallback) {
2379-
// Activity snapshots don't include message/content metadata. Reuse any
2380-
// still-active stream context captured before this workspace was backgrounded
2381-
// so compaction continue turns remain suppressible in App notifications.
2382-
this.responseCompleteCallback(
2383-
workspaceId,
2384-
"",
2385-
true,
2386-
"",
2387-
backgroundCompaction,
2388-
stoppedStreamingSnapshot.recency
2389-
);
2390-
}
2450+
// Activity snapshots don't include message/content metadata. Reuse any
2451+
// still-active stream context captured before this workspace was backgrounded
2452+
// so compaction continue turns remain suppressible in App notifications.
2453+
this.emitResponseComplete(
2454+
workspaceId,
2455+
"",
2456+
true,
2457+
"",
2458+
wasIdleCompaction
2459+
? {
2460+
hasContinueMessage: backgroundCompaction?.hasContinueMessage ?? false,
2461+
isIdle: true,
2462+
}
2463+
: backgroundCompaction,
2464+
stoppedStreamingSnapshot.recency,
2465+
false
2466+
);
23912467
}
23922468

23932469
if (isBackgroundStreamingStop) {
@@ -3118,29 +3194,6 @@ export class WorkspaceStore {
31183194
this.workspaceCreatedAt.clear();
31193195
}
31203196

3121-
/**
3122-
* Subscribe to idle compaction events.
3123-
* Callback is called when backend signals a workspace started idle compaction.
3124-
* Returns unsubscribe function.
3125-
*/
3126-
onIdleCompactionStarted(callback: (workspaceId: string) => void): () => void {
3127-
this.idleCompactionCallbacks.add(callback);
3128-
return () => this.idleCompactionCallbacks.delete(callback);
3129-
}
3130-
3131-
/**
3132-
* Notify all listeners that a workspace started idle compaction.
3133-
*/
3134-
private notifyIdleCompactionStarted(workspaceId: string): void {
3135-
for (const callback of this.idleCompactionCallbacks) {
3136-
try {
3137-
callback(workspaceId);
3138-
} catch (error) {
3139-
console.error("Error in idle compaction callback:", error);
3140-
}
3141-
}
3142-
}
3143-
31443197
/**
31453198
* Subscribe to file-modifying tool completions.
31463199
* @param listener Called with workspaceId when a file-modifying tool completes
@@ -3211,7 +3264,7 @@ export class WorkspaceStore {
32113264
}
32123265
// Wire up response complete callback for "notify on response" feature
32133266
if (this.responseCompleteCallback) {
3214-
aggregator.onResponseComplete = this.responseCompleteCallback;
3267+
this.bindAggregatorResponseCompleteCallback(aggregator);
32153268
}
32163269
this.aggregators.set(workspaceId, aggregator);
32173270
this.workspaceCreatedAt.set(workspaceId, createdAt);
@@ -3362,12 +3415,6 @@ export class WorkspaceStore {
33623415
return;
33633416
}
33643417

3365-
// Handle idle-compaction-started event from backend execution.
3366-
if ("type" in data && data.type === "idle-compaction-started") {
3367-
this.notifyIdleCompactionStarted(workspaceId);
3368-
return;
3369-
}
3370-
33713418
// Heartbeat events are no-ops for UI state - they exist only for connection liveness detection
33723419
if ("type" in data && data.type === "heartbeat") {
33733420
return;
@@ -3535,8 +3582,6 @@ function getStoreInstance(): WorkspaceStore {
35353582
* Use this for non-hook subscriptions (e.g., in useEffect callbacks).
35363583
*/
35373584
export const workspaceStore = {
3538-
onIdleCompactionStarted: (callback: (workspaceId: string) => void) =>
3539-
getStoreInstance().onIdleCompactionStarted(callback),
35403585
subscribeFileModifyingTool: (listener: (workspaceId: string) => void, workspaceId?: string) =>
35413586
getStoreInstance().subscribeFileModifyingTool(listener, workspaceId),
35423587
getFileModifyingToolMs: (workspaceId: string) =>

‎src/browser/utils/messages/StreamingMessageAggregator.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ export class StreamingMessageAggregator {
409409
messageId: string,
410410
isFinal: boolean,
411411
finalText: string,
412-
compaction?: { hasContinueMessage: boolean },
412+
compaction?: { hasContinueMessage: boolean; isIdle?: boolean },
413413
completedAt?: number | null
414414
) => void;
415415

‎src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,7 @@ export function applyWorkspaceChatEventToAggregator(
193193
isBashOutputEvent(event) ||
194194
("type" in event && event.type === "session-usage-delta") ||
195195
("type" in event && event.type === "auto-compaction-triggered") ||
196-
("type" in event && event.type === "auto-compaction-completed") ||
197-
("type" in event && event.type === "idle-compaction-started")
196+
("type" in event && event.type === "auto-compaction-completed")
198197
) {
199198
return "ignored";
200199
}

0 commit comments

Comments
 (0)