mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-07-12 21:02:17 +02:00
fix: suppress notification spam on app re-open (#623)
* fix: suppress notification spam on app re-open
- Add background_task notification category (default ON) so users can
toggle off background-task pings via Settings → Notifications
- Route notify-user builtin through notifyIfEnabled gate so the
category toggle takes effect
- Add 60s startup grace period: background-task notifications fired
within 60s of launch are suppressed, killing the reopen flood where
all queued agents complete at once
- Suppress new_email notifications for emails older than 5 min so
Gmail's startup backlog replay doesn't surface day-old mail
Fixes both issues reported by Ramnique.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: skip automatic chat_completion ping for background task agents
Background task runs were triggering "Response ready / Your agent
finished responding" on every completion. Skip the automatic
chat_completion notification when finalState.runUseCase ===
'background_task_agent' — background tasks notify explicitly via
notify-user when they have something worth surfacing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: correct notification deeplinks and skip internal agent pings
- runtime.ts: also skip chat_completion ping for knowledge_sync
useCase (agent_notes_agent was opening notes view on click)
- sync_gmail.ts: new_email notification now links to specific
email thread (rowboat://open?type=email&threadId=...)
- App.tsx: add email case to parseDeepLink, parse threadId param,
wire threadId through applyViewState, update viewStatesEqual
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: distinguish background-agent notifications from auto knowledge-sync
Per team clarification, two background types are handled differently:
- knowledge_sync (auto knowledge-graph generation): never notifies;
skips the generic chat_completion ping entirely.
- background_task_agent (user-configured agents): notifies via its
own notify-user path, gated behind the toggleable "Background
agents" category, deep-linking to the background-tasks page.
- runtime.ts: skip the generic chat_completion completion ping for
both knowledge_sync and background_task_agent (the latter notifies
via notify-user, so the generic ping would duplicate it).
- builtin-tools.ts: notify-user branches on getCurrentUseCase() —
background agents route through notifyIfEnabled('background_task')
with a bg-tasks deeplink default; chat agents notify directly.
- App.tsx: add the bg-tasks deeplink target (ViewState, parseDeepLink,
applyViewState, currentViewState).
- settings-dialog.tsx: rename the category label to "Background agents".
- runner.ts: wrap the background-task run in withUseCase so tools see
the correct use-case context.
* fix: route coding-session notifications through notifyIfEnabled
Code-mode status-tracker was calling notificationService.notify() directly,
bypassing the user's notification-category toggles. Routed both calls
through notifyIfEnabled() with correct categories:
- needs-you state → agent_permission
- idle after 30s → chat_completion
Removed now-redundant container/INotificationService plumbing.
* fix: fall back to persisted run useCase when ALS context is missing in notify-user
AsyncLocalStorage does not propagate across the background-task agent's
async generator — getCurrentUseCase() returns undefined inside notify-user,
causing the background_task_agent branch to be skipped and the Background
agents toggle to be ignored.
Fix: load persisted useCase from run record via fetchRun(ctx.runId) when
ALS is falsy. Lazy dynamic import() avoids the known module-init cycle.
Background-agent notifications now correctly respect the toggle and only
fire when the app is in the background, with deep link to the bg-tasks page.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
c0b38d46d3
commit
1bfda94f48
13 changed files with 250 additions and 22 deletions
|
|
@ -448,6 +448,20 @@ export class AgentRuntime implements IAgentRuntime {
|
|||
finalState.ingest(event);
|
||||
}
|
||||
if (finalState.getPendingPermissions().length === 0) {
|
||||
// This generic completion ping is only for real user
|
||||
// chats (copilot_chat). Skip it for:
|
||||
// - knowledge_sync: an internal, auto-running agent
|
||||
// (knowledge-graph generation) that never notifies at
|
||||
// all and has no user-facing chat to "Open".
|
||||
// - background_task_agent: a user-configured agent that
|
||||
// DOES notify, but exclusively through its own
|
||||
// notify-user path; firing this ping too would
|
||||
// duplicate that notification.
|
||||
// (The finally block still runs on this early return.)
|
||||
if (
|
||||
finalState.runUseCase === "knowledge_sync" ||
|
||||
finalState.runUseCase === "background_task_agent"
|
||||
) return;
|
||||
void notifyIfEnabled("chat_completion", {
|
||||
title: "Response ready",
|
||||
message: "Your agent finished responding.",
|
||||
|
|
|
|||
|
|
@ -99,6 +99,7 @@ import { getAccessToken } from "../../auth/tokens.js";
|
|||
import { API_URL } from "../../config/env.js";
|
||||
import type { IBrowserControlService } from "../browser-control/service.js";
|
||||
import type { INotificationService } from "../notification/service.js";
|
||||
import { notifyIfEnabled } from "../notification/notifier.js";
|
||||
// Parser libraries are loaded dynamically inside parseFile.execute()
|
||||
// to avoid pulling pdfjs-dist's DOM polyfills into the main bundle.
|
||||
// Import paths are computed so esbuild cannot statically resolve them.
|
||||
|
|
@ -1659,13 +1660,44 @@ export const BuiltinTools: z.infer<typeof BuiltinToolsSchema> = {
|
|||
return false;
|
||||
}
|
||||
},
|
||||
execute: async ({ title, message, link, actionLabel, secondaryActions }: { title?: string; message: string; link?: string; actionLabel?: string; secondaryActions?: Array<{ label: string; link: string }> }) => {
|
||||
execute: async ({ title, message, link, actionLabel, secondaryActions }: { title?: string; message: string; link?: string; actionLabel?: string; secondaryActions?: Array<{ label: string; link: string }> }, ctx?: ToolContext) => {
|
||||
try {
|
||||
const service = container.resolve<INotificationService>('notificationService');
|
||||
if (!service.isSupported()) {
|
||||
return { success: false, error: 'Notifications are not supported on this system' };
|
||||
}
|
||||
service.notify({ title, message, link, actionLabel, secondaryActions });
|
||||
let uc = getCurrentUseCase()?.useCase;
|
||||
// ALS doesn't reliably propagate across the run's async generator,
|
||||
// so when the in-context use-case is missing, fall back to the
|
||||
// persisted use case on the run record via ctx.runId.
|
||||
if (!uc && ctx?.runId) {
|
||||
try {
|
||||
const { fetchRun } = await import("../../runs/runs.js");
|
||||
const run = await fetchRun(ctx.runId);
|
||||
uc = run.useCase;
|
||||
} catch {
|
||||
// best effort — fall through to the default branch
|
||||
}
|
||||
}
|
||||
if (uc === 'background_task_agent') {
|
||||
// User-configured background agent: gate behind the
|
||||
// background_task category (toggleable), suppress the reopen
|
||||
// flood, and default the deep-link to the background tasks
|
||||
// page if the agent didn't supply its own link.
|
||||
await notifyIfEnabled('background_task', {
|
||||
title,
|
||||
message,
|
||||
link: link ?? 'rowboat://open?type=bg-tasks',
|
||||
actionLabel,
|
||||
secondaryActions,
|
||||
suppressDuringStartupGrace: true,
|
||||
onlyWhenBackground: true,
|
||||
});
|
||||
} else {
|
||||
// Regular chat (or any other) agent calling notify-user:
|
||||
// notify directly as before.
|
||||
service.notify({ title, message, link, actionLabel, secondaryActions });
|
||||
}
|
||||
return { success: true };
|
||||
} catch (error) {
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,32 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { shouldSuppressDuringStartupGrace, STARTUP_GRACE_MS } from './service.js';
|
||||
|
||||
describe('shouldSuppressDuringStartupGrace (background-task reopen flood)', () => {
|
||||
const launchedAt = 1_700_000_000_000;
|
||||
|
||||
it('suppresses a grace-eligible notification fired inside the window', () => {
|
||||
const now = launchedAt + STARTUP_GRACE_MS - 1;
|
||||
expect(shouldSuppressDuringStartupGrace({ suppressDuringStartupGrace: true }, launchedAt, now)).toBe(true);
|
||||
});
|
||||
|
||||
it('lets a grace-eligible notification through once the window has passed', () => {
|
||||
const now = launchedAt + STARTUP_GRACE_MS;
|
||||
expect(shouldSuppressDuringStartupGrace({ suppressDuringStartupGrace: true }, launchedAt, now)).toBe(false);
|
||||
});
|
||||
|
||||
it('never suppresses a notification that is not grace-eligible', () => {
|
||||
const now = launchedAt + 1; // well inside the window
|
||||
expect(shouldSuppressDuringStartupGrace({ suppressDuringStartupGrace: false }, launchedAt, now)).toBe(false);
|
||||
expect(shouldSuppressDuringStartupGrace({}, launchedAt, now)).toBe(false);
|
||||
});
|
||||
|
||||
it('respects a custom grace window', () => {
|
||||
const customWindow = 5_000;
|
||||
expect(
|
||||
shouldSuppressDuringStartupGrace({ suppressDuringStartupGrace: true }, launchedAt, launchedAt + 4_999, customWindow),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldSuppressDuringStartupGrace({ suppressDuringStartupGrace: true }, launchedAt, launchedAt + 5_000, customWindow),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -12,9 +12,40 @@ export interface NotifyInput {
|
|||
* regardless of focus (e.g. an agent permission request that blocks a run).
|
||||
*/
|
||||
onlyWhenBackground?: boolean;
|
||||
/**
|
||||
* When true, the notification is suppressed if it fires within the startup
|
||||
* grace window (see STARTUP_GRACE_MS). This exists for notifications that a
|
||||
* just-launched app can emit in a burst — most notably background-task
|
||||
* completions: when the app reopens after being closed, every task that was
|
||||
* queued while it was down completes at once and would otherwise flood the
|
||||
* user. Fresh, user-driven activity happens after the window closes.
|
||||
*/
|
||||
suppressDuringStartupGrace?: boolean;
|
||||
}
|
||||
|
||||
export interface INotificationService {
|
||||
isSupported(): boolean;
|
||||
notify(input: NotifyInput): void;
|
||||
}
|
||||
|
||||
/**
|
||||
* How long after launch grace-eligible notifications stay suppressed. Long
|
||||
* enough to swallow the reopen burst of queued background tasks, short enough
|
||||
* that a task genuinely finishing right after launch still pings the user.
|
||||
*/
|
||||
export const STARTUP_GRACE_MS = 60_000;
|
||||
|
||||
/**
|
||||
* Pure decision for the startup grace gate, kept out of the Electron service so
|
||||
* it can be unit-tested without an Electron runtime. Returns true when the
|
||||
* notification should be dropped because it is grace-eligible and we are still
|
||||
* inside the window measured from `launchedAt`.
|
||||
*/
|
||||
export function shouldSuppressDuringStartupGrace(
|
||||
input: Pick<NotifyInput, "suppressDuringStartupGrace">,
|
||||
launchedAt: number,
|
||||
now: number = Date.now(),
|
||||
graceWindowMs: number = STARTUP_GRACE_MS,
|
||||
): boolean {
|
||||
return Boolean(input.suppressDuringStartupGrace) && now - launchedAt < graceWindowMs;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { getBackgroundTaskAgentModel } from '../models/defaults.js';
|
|||
import { extractAgentResponse, waitForRunCompletion } from '../agents/utils.js';
|
||||
import { buildTriggerBlock } from '../agents/build-trigger-block.js';
|
||||
import { backgroundTaskBus } from './bus.js';
|
||||
import { withUseCase } from '../analytics/use_case.js';
|
||||
|
||||
const log = new PrefixLogger('BgTask:Agent');
|
||||
|
||||
|
|
@ -183,7 +184,18 @@ export async function runBackgroundTask(
|
|||
});
|
||||
|
||||
try {
|
||||
await createMessage(runId, buildMessage(slug, task, trigger, context, codeProject));
|
||||
// Establish the use-case context for the whole run so every tool the
|
||||
// agent calls (notably notify-user) reads `background_task_agent` via
|
||||
// getCurrentUseCase(). createMessage synchronously fires
|
||||
// agentRuntime.trigger(), so the detached run loop — and the tool
|
||||
// calls within it — inherit this AsyncLocalStorage context. (The
|
||||
// runtime's own enterUseCase runs inside an async generator and
|
||||
// doesn't reliably propagate to tool execution, so we set it here at
|
||||
// the trigger point instead.)
|
||||
await withUseCase(
|
||||
{ useCase: 'background_task_agent', subUseCase: trigger },
|
||||
() => createMessage(runId, buildMessage(slug, task, trigger, context, codeProject)),
|
||||
);
|
||||
await waitForRunCompletion(runId, { throwOnError: true });
|
||||
const summary = await extractAgentResponse(runId);
|
||||
|
||||
|
|
|
|||
|
|
@ -2,9 +2,8 @@ import z from 'zod';
|
|||
import { RunEvent } from '@x/shared/dist/runs.js';
|
||||
import type { IBus } from '../../application/lib/bus.js';
|
||||
import type { ICodeSessionsRepo } from './repo.js';
|
||||
import type { INotificationService } from '../../application/notification/service.js';
|
||||
import { notifyIfEnabled } from '../../application/notification/notifier.js';
|
||||
import type { CodeSessionStatus, CodeSession } from '@x/shared/dist/code-sessions.js';
|
||||
import container from '../../di/container.js';
|
||||
|
||||
export type StatusListener = (sessionId: string, status: CodeSessionStatus) => void;
|
||||
|
||||
|
|
@ -107,17 +106,16 @@ export class CodeSessionStatusTracker {
|
|||
}
|
||||
|
||||
private async notify(sessionId: string, previous: CodeSessionStatus, next: CodeSessionStatus): Promise<void> {
|
||||
let notificationService: INotificationService;
|
||||
try {
|
||||
notificationService = container.resolve<INotificationService>('notificationService');
|
||||
} catch {
|
||||
return; // not registered (e.g. tests)
|
||||
}
|
||||
if (!notificationService.isSupported()) return;
|
||||
// Route through notifyIfEnabled so the user's notification-category
|
||||
// toggles are honoured — a coding agent asking for approval maps to
|
||||
// `agent_permission`, and one finishing its turn maps to
|
||||
// `chat_completion`. notifyIfEnabled also resolves the service, checks
|
||||
// platform support, and swallows errors, so a disabled toggle, missing
|
||||
// service (e.g. tests), or unsupported platform all no-op safely.
|
||||
const session = await this.codeSessionsRepo.get(sessionId);
|
||||
const title = session?.title ?? 'Coding session';
|
||||
if (next === 'needs-you') {
|
||||
notificationService.notify({
|
||||
await notifyIfEnabled('agent_permission', {
|
||||
title,
|
||||
message: 'The coding agent needs your approval.',
|
||||
});
|
||||
|
|
@ -126,7 +124,7 @@ export class CodeSessionStatusTracker {
|
|||
// the user has plausibly moved on to something else.
|
||||
const since = this.busySince.get(sessionId);
|
||||
if (since !== undefined && Date.now() - since > 30_000) {
|
||||
notificationService.notify({
|
||||
await notifyIfEnabled('chat_completion', {
|
||||
title,
|
||||
message: 'The coding agent finished its turn.',
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
isEmailTooOldToNotify,
|
||||
NEW_EMAIL_MAX_AGE_MS,
|
||||
sanitizeReplyBodyForGmailReply,
|
||||
stripGmailQuotedReplyHtml,
|
||||
stripGmailQuotedReplyText,
|
||||
|
|
@ -40,3 +42,31 @@ describe('Gmail reply body sanitization', () => {
|
|||
expect(result.bodyHtml).toBe('<p>Sounds good, thanks.</p>');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isEmailTooOldToNotify (stale backlog suppression)', () => {
|
||||
const now = 1_700_000_000_000;
|
||||
|
||||
it('suppresses emails older than the freshness window', () => {
|
||||
const old = now - NEW_EMAIL_MAX_AGE_MS - 1;
|
||||
expect(isEmailTooOldToNotify(old, now)).toBe(true);
|
||||
});
|
||||
|
||||
it('notifies for emails within the freshness window', () => {
|
||||
const recent = now - (NEW_EMAIL_MAX_AGE_MS - 1);
|
||||
expect(isEmailTooOldToNotify(recent, now)).toBe(false);
|
||||
});
|
||||
|
||||
it('notifies for emails exactly at the window boundary', () => {
|
||||
expect(isEmailTooOldToNotify(now - NEW_EMAIL_MAX_AGE_MS, now)).toBe(false);
|
||||
});
|
||||
|
||||
it('notifies when the email date is unknown (dateMs === 0)', () => {
|
||||
// 0 means snapshotDateMs could not parse a date; err toward notifying
|
||||
// rather than silently dropping genuinely-new mail.
|
||||
expect(isEmailTooOldToNotify(0, now)).toBe(false);
|
||||
});
|
||||
|
||||
it('notifies for a brand-new email (dateMs === now)', () => {
|
||||
expect(isEmailTooOldToNotify(now, now)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -221,20 +221,40 @@ function summarizeGmailSync(threads: SyncedThread[]): string {
|
|||
return lines.join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* A "new email" notification for a message older than this is treated as a
|
||||
* stale backlog item — e.g. Gmail replaying history after the app reopens from
|
||||
* a long offline period — and suppressed, so a reopen doesn't surface day-old
|
||||
* mail as if it just arrived.
|
||||
*/
|
||||
export const NEW_EMAIL_MAX_AGE_MS = 5 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* True when an email is too old to be worth a "new email" ping. A `dateMs` of 0
|
||||
* means the age couldn't be determined, in which case we err toward notifying
|
||||
* rather than risk silently dropping genuinely-new mail.
|
||||
*/
|
||||
export function isEmailTooOldToNotify(dateMs: number, now: number = Date.now()): boolean {
|
||||
return dateMs > 0 && now - dateMs > NEW_EMAIL_MAX_AGE_MS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fire one OS notification per genuinely-new email thread. Only ever called
|
||||
* from the partial-sync (incremental) path, so the first-time connect — which
|
||||
* goes through fullSync — never notifies. Suppressed while the app is focused.
|
||||
* goes through fullSync — never notifies. Suppressed while the app is focused,
|
||||
* and for stale backlog (see isEmailTooOldToNotify).
|
||||
*/
|
||||
function notifyNewEmails(threads: SyncedThread[]): void {
|
||||
const now = Date.now();
|
||||
for (const { threadId } of threads) {
|
||||
const snapshot = readCachedSnapshot(threadId)?.snapshot;
|
||||
if (snapshot && isEmailTooOldToNotify(snapshotDateMs(snapshot), now)) continue;
|
||||
const subject = snapshot?.subject?.trim() || '(no subject)';
|
||||
const from = snapshot?.from?.trim();
|
||||
void notifyIfEnabled('new_email', {
|
||||
title: from ? `New email from ${from}` : 'New email',
|
||||
message: subject,
|
||||
link: 'rowboat://open?type=chat',
|
||||
link: `rowboat://open?type=email&threadId=${threadId}`,
|
||||
actionLabel: 'Open',
|
||||
onlyWhenBackground: true,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -6,17 +6,20 @@ import { z } from 'zod';
|
|||
* - chat_completion: an agent finished generating a response
|
||||
* - new_email: a new email arrived during incremental Gmail sync
|
||||
* - agent_permission: an agent is requesting permission to run a tool
|
||||
* - background_task: a background task agent pinged via the notify-user tool
|
||||
*/
|
||||
export const NotificationCategorySchema = z.enum([
|
||||
'chat_completion',
|
||||
'new_email',
|
||||
'agent_permission',
|
||||
'background_task',
|
||||
]);
|
||||
|
||||
export const NotificationCategoriesSchema = z.object({
|
||||
chat_completion: z.boolean(),
|
||||
new_email: z.boolean(),
|
||||
agent_permission: z.boolean(),
|
||||
background_task: z.boolean(),
|
||||
});
|
||||
|
||||
export const NotificationSettingsSchema = z.object({
|
||||
|
|
@ -28,6 +31,7 @@ export const DEFAULT_NOTIFICATION_SETTINGS: NotificationSettings = {
|
|||
chat_completion: true,
|
||||
new_email: true,
|
||||
agent_permission: true,
|
||||
background_task: true,
|
||||
},
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue