🤖 fix: suppress browser notifications for idle compaction completions#2554
🤖 fix: suppress browser notifications for idle compaction completions#2554
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30b33f3ce2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
30b33f3 to
ccb274f
Compare
|
@codex review Addressed: now derive idle-compaction state from both the previous (start) and current (stop) snapshots, so UI reconnects that miss the transient flag on the start snapshot still suppress idle compaction notifications. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb274fb8c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Idle compaction is background maintenance, not user-initiated work. Previously, the frontend tracked idle compaction via a Set populated from the idle-compaction-started chat event, but onChat only subscribes for the active workspace — so the event never arrived for truly background workspaces (the main idle compaction target). Fix: thread isIdleCompaction through the activity snapshot, which is the only data channel reaching the frontend for ALL workspaces. The backend tags the snapshot during updateStreamingStatus when an idle compaction is in progress. The frontend reads the flag on the streaming true→false transition and suppresses the notification. Changes: - Add isIdleCompaction field to WorkspaceActivitySnapshotSchema - Backend: track idle compaction Set, enrich snapshot, clear on stop - Frontend: remove broken idleCompactingWorkspaces Set, read from previous snapshot's isIdleCompaction flag instead - App.tsx: early-return before notification when compaction?.isIdle
ccb274f to
20f0834
Compare
|
@codex review Addressed: moved |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20f0834c51
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
- Clarify WorkspaceStore comment about when isIdleCompaction is present - Simplify background compaction payload construction on stream stop - Remove unused idle-compaction-started event plumbing across frontend/backend - Update idle compaction dispatch tests to match removed started-event emission --- _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 -->
|
@codex review Latest idle-compaction cleanup is pushed (including dead |
Guard idle-compaction marker assignment behind a post-dispatch session.isBusy() check. If sendMessage succeeds without an active stream (startup-abort or ultra-fast completion), clear any stale marker so later user streams are never misclassified as idle compaction. Also add regression tests for both active-stream and no-stream success paths. --- _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 -->
|
@codex review Addressed stale idle-marker race:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e36d9cbc88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
When a compaction stream ends with a queued follow-up message, mark the completion as hasContinueMessage=true before forwarding onResponseComplete. This prevents an intermediate "Compaction complete" notification and keeps notification delivery focused on the follow-up response. Also adds a WorkspaceStore regression test covering active compaction + queued follow-up. --- _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 -->
|
@codex review Added another notification suppression fix:
|
Move idle-compaction marker cleanup for streaming=false transitions into updateStreamingStatus finally block so extension metadata failures cannot leak isIdleCompaction state into later user streams. Adds a regression test that forces setStreaming failure and verifies the marker is still cleared. --- _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 -->
|
@codex review Addressed the new idle-marker leak case:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ac1363262
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Restrict updateStreamingStatus idle tagging to streaming=false snapshots so follow-up non-idle turns cannot inherit stale isIdleCompaction metadata from prior maintenance turns. Also adds a regression test for streaming=true behavior while the idle marker is set. --- _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 -->
|
@codex review Addressed stream-stop tagging feedback:
|
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
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-startedchat event. That signal is unreliable for true background work becauseonChatis 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
Activity snapshot contract
isIdleCompactiontoWorkspaceActivitySnapshotSchema.Backend idle marker lifecycle (
WorkspaceService)idleCompactingWorkspaces.isIdleCompactiononly onstreaming=falsesnapshots (stop snapshots), not onstreaming=truesnapshots.streaming=falsetransitions infinally, so metadata write failures cannot leak state into future turns.Frontend completion suppression (
WorkspaceStore+App)streaming true→false+ recency advance) for background completion detection.previous || snapshotfor reconnect/restore resilience.hasContinueMessage=trueso App suppresses intermediate "Compaction complete" notifications.App.tsxsuppresses notifications forcompaction?.isIdleandcompaction?.hasContinueMessage.Cleanup
idle-compaction-startedplumbing across backend/frontend:Tests
setStreamingfails on stream-stopstreaming=truesnapshots are never idle-taggedValidation
make static-checkbun test src/browser/stores/WorkspaceStore.test.tsbun test src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.tsbun test src/node/services/workspaceService.test.tsmake typecheckRisks
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