Skip to content

Fix notification spam and clarify agent ready state#148

Merged
TraderSamwise merged 4 commits into
masterfrom
chore/notifications-next-followup
Jun 15, 2026
Merged

Fix notification spam and clarify agent ready state#148
TraderSamwise merged 4 commits into
masterfrom
chore/notifications-next-followup

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • disable external desktop/mobile notification delivery during tests
  • mark session notifications read and clear generic needs-input attention when viewing an agent
  • add a low-emphasis ready state for live agents that are not actively working

Verification

  • yarn vitest run src/session-viewed.test.ts src/session-semantics.test.ts src/metadata-server.test.ts src/tui/screens/dashboard-renderers.test.ts
  • pre-push hook: yarn typecheck && yarn lint && yarn test
  • yarn build

Summary by CodeRabbit

  • New Features
    • Added "ready" session status for idle running sessions with no active work.
    • Implemented interaction request deduplication to prevent duplicate alerts for the same session and payload.
    • Added configuration options to control automatic notification behavior when viewing sessions (markReadOnView, clearNeedsInputOnView, clearFormalInteractionsOnView).
    • Added support for disabling external and desktop notifications via environment variables.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6dcfc379-a894-4502-88de-31d5ca1ab61c

📥 Commits

Reviewing files that changed from the base of the PR and between 270eb48 and 3b73344.

📒 Files selected for processing (23)
  • src/config.test.ts
  • src/config.ts
  • src/desktop-notifier.test.ts
  • src/desktop-notifier.ts
  • src/external-notifications.ts
  • src/interaction-requests.ts
  • src/metadata-server.interaction.test.ts
  • src/metadata-server.test.ts
  • src/metadata-server.ts
  • src/mobile-push-bridge.test.ts
  • src/mobile-push-bridge.ts
  • src/multiplexer/dashboard-control.ts
  • src/multiplexer/session-launch.ts
  • src/notify.test.ts
  • src/notify.ts
  • src/session-semantics.test.ts
  • src/session-semantics.ts
  • src/session-viewed.test.ts
  • src/session-viewed.ts
  • src/tui/render/theme.ts
  • src/tui/screens/dashboard-renderers.test.ts
  • src/tui/screens/dashboard-renderers.ts
  • src/vitest.setup.ts

📝 Walkthrough

Walkthrough

This PR introduces a markSessionViewed module that consolidates session-view bookkeeping (marking notifications read, clearing attention, resetting unseen count) driven by configurable NotificationConfig flags. It wires markSessionViewed across MetadataServer endpoints and the multiplexer. Interaction alerts gain deduplication via a deterministic dedupeKey fingerprint and a 60-second cooldown. A new "ready" semantic session state is added end-to-end. An externalNotificationsDisabled utility gates all desktop and mobile push notification paths.

Changes

Session-viewed tracking, interaction deduplication, "ready" state, and notification gating

Layer / File(s) Summary
externalNotificationsDisabled utility and notification gating
src/external-notifications.ts, src/notify.ts, src/desktop-notifier.ts, src/mobile-push-bridge.ts, src/vitest.setup.ts, src/notify.test.ts, src/desktop-notifier.test.ts, src/mobile-push-bridge.test.ts
New externalNotificationsDisabled function checks two env vars; early-return guards added to send, sendDesktopNotification, sendDesktopNotificationAndWait, and forwardAlertToMobilePush; DesktopNotificationTransport gains "disabled"; global Vitest setup sets the env var; all paths tested.
markSessionViewed module and NotificationConfig view fields
src/config.ts, src/session-viewed.ts, src/config.test.ts, src/session-viewed.test.ts
NotificationConfig adds markReadOnView, clearNeedsInputOnView, clearFormalInteractionsOnView with defaults. New session-viewed.ts loads config, conditionally reads notifications, clears or preserves derived.attention, resets derived.unseenCount, and returns { notificationsRead, attentionCleared }.
markSessionViewed wired into server endpoints and multiplexer
src/metadata-server.ts, src/multiplexer/dashboard-control.ts, src/multiplexer/session-launch.ts, src/metadata-server.test.ts
Replaces tracker.markSeen/markNotificationsRead with markSessionViewed in markActiveWindowFocused, /control/open-notification-target, /control/focus-window, /control/switch-next|prev|attention, /mark-seen, openLiveTmuxWindowForEntry, and focusSession. Integration test expanded with teammate derived-state setup and cleared-attention assertions.
Interaction request deduplication via deterministic dedupe key
src/interaction-requests.ts, src/metadata-server.ts, src/metadata-server.interaction.test.ts
InteractionRequest and RegisterInteractionInput gain optional dedupeKey; register() returns existing pending request on key collision via findPendingByDedupeKey(). metadata-server.ts adds interactionDedupeKey fingerprinting {sessionId, type, summary, payload} and applies it with cooldownMs: 60_000 in both interaction paths.
"ready" session semantic state through theme and dashboard rendering
src/session-semantics.ts, src/tui/render/theme.ts, src/tui/screens/dashboard-renderers.ts, src/session-semantics.test.ts, src/tui/screens/dashboard-renderers.test.ts
SessionUserLabel adds "ready"; deriveSessionSemantics maps status === "running" without active task to "ready" instead of "working". StatusKind and statusDot support "ready" with glyph and muted tone. Dashboard renderers add "ready" to labels, time anchor, rank, count, and status dot.

Sequence Diagram(s)

sequenceDiagram
  participant UI as TUI / Control Client
  participant MS as MetadataServer
  participant SV as markSessionViewed
  participant NS as NotificationStore

  rect rgba(100, 149, 237, 0.5)
    Note over UI,NS: Session Viewed Flow
    UI->>MS: /control/open-notification-target or /control/switch-*
    MS->>SV: markSessionViewed(sessionId)
    SV->>NS: markNotificationsRead (if markReadOnView)
    SV->>MS: updateSessionMetadata(unseenCount=0, attention=normal)
    SV-->>MS: { notificationsRead, attentionCleared }
  end

  rect rgba(180, 100, 100, 0.5)
    Note over UI,NS: Interaction Deduplication Flow
    UI->>MS: POST /agents/interaction (1st call)
    MS->>MS: interactionDedupeKey(sessionId,type,summary,payload)
    MS->>MS: InteractionRegistry.register({ dedupeKey })
    MS-->>UI: requestId=X, alert emitted
    UI->>MS: POST /agents/interaction (2nd identical call)
    MS->>MS: findPendingByDedupeKey → existing request
    MS-->>UI: same requestId=X, no new alert
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • TraderSamwise/aimux#84: Modifies the same notifyAlert/forwardAlertToMobilePush integration points in src/notify.ts and src/mobile-push-bridge.ts.
  • TraderSamwise/aimux#135: Adds StatusKind/statusDot theme primitives that this PR extends with the "ready" variant.
  • TraderSamwise/aimux#124: Modifies notifyAlert control flow in src/notify.ts in a directly adjacent way to this PR's external-notifications guard.

Poem

🐇 A session once yelled, "I need your attention!"
But now when you look, it clears with intention.
"ready" appears where "working" once stood,
Dedupe keys fire once — behaving as they should.
And when notifications sleep, the env var says "shh" —
The rabbit hops quietly, no desktop buzz! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the two main changes: fixing notification spam and clarifying the agent ready state. The title directly aligns with the PR's primary objectives of disabling external notifications during tests and introducing a 'ready' status for idle agents.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/notifications-next-followup

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jun 15, 2026 6:39pm

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Sub-agent review finding resolved:

  • src/metadata-server.ts: duplicate formal interaction requests could create hidden pending requests while alert dedupe suppressed the duplicate notification. Fixed in 3b73344 by deduping pending interactions in InteractionRegistry.register using the same interaction fingerprint, so duplicate callers reuse the visible pending request. Added endpoint coverage asserting repeated requests share one id and leave only one pending interaction.

@TraderSamwise
TraderSamwise merged commit 2b65110 into master Jun 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant