diff --git a/src/main/index.ts b/src/main/index.ts index 1f15f9c4..3a2df714 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -43,7 +43,6 @@ import { join } from 'path'; import { cleanupEditorState, setEditorMainWindow } from './ipc/editor'; import { initializeIpcHandlers, removeIpcHandlers } from './ipc/handlers'; -import { showTeamNativeNotification } from './ipc/teams'; import { startEventLoopLagMonitor } from './services/infrastructure/EventLoopLagMonitor'; import { HttpServer } from './services/infrastructure/HttpServer'; import { TeamInboxReader } from './services/team/TeamInboxReader'; @@ -154,9 +153,7 @@ function extractNotificationContent(text: string): { summary: string; body: stri } async function notifyNewInboxMessages(teamName: string, detail: string): Promise { - // Check global toggle const config = configManager.getConfig(); - if (!config.notifications.enabled) return; // Skip orphaned team directories without config.json (e.g., "default"). // Claude Code may write to these when its internal teamContext is lost after session resume. @@ -172,15 +169,19 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise if (!match) return; const memberName = match[1]; - // Determine inbox type and check per-inbox toggle + // Determine inbox type and per-type toggle state. + // Storage is always unconditional; toggles only suppress the OS toast. const leadName = teamDataService ? await teamDataService.getLeadMemberName(teamName) : null; const isLeadInbox = leadName !== null && memberName === leadName; const isUserInbox = memberName === 'user'; - if (isLeadInbox && !config.notifications.notifyOnLeadInbox) return; - if (isUserInbox && !config.notifications.notifyOnUserInbox) return; if (!isLeadInbox && !isUserInbox) return; + const suppressToast = + !config.notifications.enabled || + (isLeadInbox && !config.notifications.notifyOnLeadInbox) || + (isUserInbox && !config.notifications.notifyOnUserInbox); + const key = `${teamName}:${memberName}`; try { @@ -205,7 +206,8 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise const teamDisplayName = await resolveTeamDisplayName(teamName); - for (const msg of newMessages) { + for (let i = 0; i < newMessages.length; i++) { + const msg = newMessages[i]; // Skip messages sent from our own UI if (msg.source && suppressedSources.has(msg.source)) continue; // Skip internal coordination noise (idle_notification, shutdown_*, etc.) @@ -214,12 +216,20 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise const fromLabel = msg.from || 'Unknown'; const extracted = extractNotificationContent(msg.text); const summary = msg.summary || extracted.summary; + const msgId = msg.timestamp ?? String(prevCount + i); - showTeamNativeNotification({ - title: teamDisplayName, - subtitle: `${fromLabel}: ${summary}`, - body: extracted.body, - }); + void notificationManager + .addTeamNotification({ + teamEventType: isLeadInbox ? 'lead_inbox' : 'user_inbox', + teamName, + teamDisplayName, + from: fromLabel, + summary, + body: extracted.body, + dedupeKey: `inbox:${teamName}:${memberName}:${msgId}`, + suppressToast, + }) + .catch(() => undefined); } } catch (error) { logger.warn(`Failed to check inbox messages for ${key}:`, error); @@ -232,8 +242,7 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise */ async function notifyNewSentMessages(teamName: string): Promise { const config = configManager.getConfig(); - if (!config.notifications.enabled) return; - if (!config.notifications.notifyOnUserInbox) return; + const suppressToast = !config.notifications.enabled || !config.notifications.notifyOnUserInbox; try { const messages = await sentMessagesStore.readMessages(teamName); @@ -256,7 +265,8 @@ async function notifyNewSentMessages(teamName: string): Promise { const teamDisplayName = await resolveTeamDisplayName(teamName); - for (const msg of newMessages) { + for (let i = 0; i < newMessages.length; i++) { + const msg = newMessages[i]; // Skip messages sent from our own UI if (msg.source && suppressedSources.has(msg.source)) continue; // Skip internal coordination noise @@ -266,11 +276,18 @@ async function notifyNewSentMessages(teamName: string): Promise { const extracted = extractNotificationContent(msg.text); const summary = msg.summary || extracted.summary; - showTeamNativeNotification({ - title: teamDisplayName, - subtitle: `${fromLabel}: ${summary}`, - body: extracted.body, - }); + void notificationManager + .addTeamNotification({ + teamEventType: 'user_inbox', + teamName, + teamDisplayName, + from: fromLabel, + summary, + body: extracted.body, + dedupeKey: `sent:${teamName}:${msg.timestamp ?? String(prevCount + i)}`, + suppressToast, + }) + .catch(() => undefined); } } catch (error) { logger.warn(`Failed to check sent messages for ${teamName}:`, error); diff --git a/src/main/ipc/teams.ts b/src/main/ipc/teams.ts index 20919acf..bcd9964c 100644 --- a/src/main/ipc/teams.ts +++ b/src/main/ipc/teams.ts @@ -1,5 +1,3 @@ -import { randomUUID } from 'node:crypto'; - import { setCurrentMainOp } from '@main/services/infrastructure/EventLoopLagMonitor'; import { getAppIconPath } from '@main/utils/appIcon'; import { stripMarkdown } from '@main/utils/textFormatting'; @@ -79,10 +77,6 @@ import { validateTeamName, } from './guards'; -/** Track rate limit message keys already notified to avoid duplicate OS notifications across refreshes. */ -const notifiedRateLimitKeys = new Set(); -const RATE_LIMIT_KEYS_MAX = 500; - import { TeamAttachmentStore } from '../services/team/TeamAttachmentStore'; import { TeamTaskAttachmentStore } from '../services/team/TeamTaskAttachmentStore'; @@ -130,7 +124,17 @@ import type { const logger = createLogger('IPC:teams'); /** - * Check messages for rate limit indicators and fire native notifications for new ones. + * In-memory set of rate-limit message keys already processed. + * Independent of NotificationManager storage — survives notification deletion/pruning. + * Without this, deleted rate-limit notifications would re-appear on next getData() scan. + */ +const seenRateLimitKeys = new Set(); +const SEEN_RATE_LIMIT_KEYS_MAX = 500; + +/** + * Check messages for rate limit indicators and fire notifications for new ones. + * Uses both in-memory seenRateLimitKeys (to prevent resurrection after deletion) + * and NotificationManager dedupeKey (to prevent storage duplicates). */ function checkRateLimitMessages( messages: readonly { messageId?: string; from: string; text: string; timestamp: string }[], @@ -142,33 +146,29 @@ function checkRateLimitMessages( if (msg.from === 'user') continue; if (!isRateLimitMessage(msg.text)) continue; - // Prefix key with teamName to avoid collisions across teams const rawKey = msg.messageId ?? `${msg.from}:${msg.timestamp}`; - const key = `${teamName}:${rawKey}`; - if (notifiedRateLimitKeys.has(key)) continue; - notifiedRateLimitKeys.add(key); + const dedupeKey = `rate-limit:${teamName}:${rawKey}`; - // Prevent unbounded memory growth - if (notifiedRateLimitKeys.size > RATE_LIMIT_KEYS_MAX) { - const first = notifiedRateLimitKeys.values().next().value!; - notifiedRateLimitKeys.delete(first); + // In-memory guard: prevents resurrection after user deletes the notification + if (seenRateLimitKeys.has(dedupeKey)) continue; + seenRateLimitKeys.add(dedupeKey); + + // Evict oldest entries to prevent unbounded growth + if (seenRateLimitKeys.size > SEEN_RATE_LIMIT_KEYS_MAX) { + const first = seenRateLimitKeys.values().next().value; + if (first) seenRateLimitKeys.delete(first); } void NotificationManager.getInstance() - .addError({ - id: randomUUID(), - timestamp: Date.now(), - sessionId: `team:${teamName}`, - projectId: teamName, - filePath: '', - source: 'rate-limit', - message: `[${msg.from}] ${msg.text.slice(0, 200)}`, - triggerColor: 'red', - triggerName: 'Rate Limit', - context: { - projectName: teamDisplayName, - cwd: projectPath, - }, + .addTeamNotification({ + teamEventType: 'rate_limit', + teamName, + teamDisplayName, + from: msg.from, + summary: `Rate limit: ${msg.from}`, + body: msg.text.slice(0, 200), + dedupeKey, + projectPath, }) .catch(() => undefined); } @@ -1950,19 +1950,39 @@ async function handleShowMessageNotification( if (!d.teamDisplayName || !d.from || !d.body) { return { success: false, error: 'Missing required fields (teamDisplayName, from, body)' }; } + if (!d.teamName) { + return { + success: false, + error: 'Missing required field: teamName (needed for deep-link navigation)', + }; + } + + // Route through NotificationManager for unified storage + native toast. + // dedupeKey is required from renderer — built from stable identifiers (taskId, teamName, etc.) + const dedupeKey = + d.dedupeKey ?? `msg:${d.teamName}:${d.from}:${d.summary ?? d.body.slice(0, 50)}`; + + void NotificationManager.getInstance() + .addTeamNotification({ + teamEventType: d.teamEventType ?? 'task_clarification', + teamName: d.teamName, + teamDisplayName: d.teamDisplayName, + from: d.from, + to: d.to, + summary: d.summary ?? `${d.from} → ${d.to ?? 'team'}`, + body: d.body, + dedupeKey, + suppressToast: d.suppressToast, + }) + .catch(() => undefined); - showTeamNativeNotification({ - title: d.teamDisplayName, - subtitle: d.summary ?? `${d.from} → ${d.to ?? 'team'}`, - body: d.body, - }); return { success: true, data: undefined }; } /** * Show a native OS notification for a team event. - * Respects user's notification settings (enabled, snoozed). - * Cross-platform: macOS, Linux, Windows via Electron Notification API. + * @deprecated Use NotificationManager.addTeamNotification() instead for unified storage + toast. + * Kept for backward compatibility with any remaining callers. */ export function showTeamNativeNotification(opts: { title: string; diff --git a/src/main/services/error/ErrorMessageBuilder.ts b/src/main/services/error/ErrorMessageBuilder.ts index 6f3ad900..1aaf2eed 100644 --- a/src/main/services/error/ErrorMessageBuilder.ts +++ b/src/main/services/error/ErrorMessageBuilder.ts @@ -49,6 +49,17 @@ export interface DetectedError { triggerId?: string; /** Human-readable name of the trigger that produced this notification */ triggerName?: string; + /** Notification domain: 'error' (default/undefined) or 'team' */ + category?: 'error' | 'team'; + /** For team notifications: specific event sub-type */ + teamEventType?: + | 'rate_limit' + | 'lead_inbox' + | 'user_inbox' + | 'task_clarification' + | 'task_status_change'; + /** Explicit key for storage deduplication. Two notifications with the same dedupeKey won't be stored twice. */ + dedupeKey?: string; /** Additional context about the error */ context: { /** Human-readable project name */ diff --git a/src/main/services/infrastructure/NotificationManager.ts b/src/main/services/infrastructure/NotificationManager.ts index 4e03fc6b..413ee08f 100644 --- a/src/main/services/infrastructure/NotificationManager.ts +++ b/src/main/services/infrastructure/NotificationManager.ts @@ -1,13 +1,16 @@ /** - * NotificationManager service - Manages native macOS notifications and error history. + * NotificationManager service - Manages native notifications and notification history. * * Responsibilities: - * - Store error history at ~/.claude/claude-devtools-notifications.json (max 100 entries) + * - Store notification history at ~/.claude/claude-devtools-notifications.json (max 100 entries) * - Show native notifications using Electron's Notification API (cross-platform) - * - Implement throttling (5 seconds per unique error hash) - * - Respect config.notifications.enabled and snoozedUntil - * - Filter errors matching ignoredRegex patterns - * - Filter errors from ignoredProjects + * - Two adapters: addError() for error notifications, addTeamNotification() for team events + * - Shared internal pipeline: storeNotification() for unconditional storage + IPC emission + * - Two-level dedup: dedupeKey for storage dedup, toast throttle (5s) for native toasts + * - Storage is unconditional — enabled/snoozed only affect native OS toasts + * - Respect config.notifications.enabled and snoozedUntil for toasts + * - Filter errors matching ignoredRegex patterns (error-specific) + * - Filter errors from ignoredProjects (error-specific) * - Auto-prune notifications over 100 on startup * - Emit IPC events to renderer: notification:new, notification:updated */ @@ -24,6 +27,10 @@ import * as path from 'path'; import { type DetectedError } from '../error/ErrorMessageBuilder'; const logger = createLogger('Service:NotificationManager'); +import { + buildDetectedErrorFromTeam, + type TeamNotificationPayload, +} from '@main/utils/teamNotificationBuilder'; import { projectPathResolver } from '../discovery/ProjectPathResolver'; import { gitIdentityResolver } from '../parsing/GitIdentityResolver'; @@ -31,6 +38,8 @@ import { ConfigManager } from './ConfigManager'; // Re-export DetectedError for backward compatibility export type { DetectedError }; +// Re-export team notification types for callers +export type { TeamNotificationPayload, TeamEventType } from '@main/utils/teamNotificationBuilder'; /** * Stored notification with read status. @@ -236,18 +245,19 @@ export class NotificationManager extends EventEmitter { } /** - * Checks if an error should be throttled. + * Checks if a native toast should be throttled. + * Uses dedupeKey if present, else falls back to projectId:message hash. */ - private isThrottled(error: DetectedError): boolean { - const hash = this.generateErrorHash(error); - const lastSeen = this.throttleMap.get(hash); + private isToastThrottled(error: DetectedError): boolean { + const key = error.dedupeKey ?? this.generateErrorHash(error); + const lastSeen = this.throttleMap.get(key); if (lastSeen && Date.now() - lastSeen < THROTTLE_MS) { return true; } // Update throttle map - this.throttleMap.set(hash, Date.now()); + this.throttleMap.set(key, Date.now()); // Clean up old entries periodically this.cleanupThrottleMap(); @@ -349,81 +359,90 @@ export class NotificationManager extends EventEmitter { return ignoredRepositories.includes(identity.id); } - /** - * Determines if an error should generate a notification. - */ - private async shouldNotify(error: DetectedError): Promise { - // Check if notifications are enabled - if (!this.areNotificationsEnabled()) { - return false; - } - - // Check if error is from an ignored repository - if (await this.isFromIgnoredRepository(error)) { - return false; - } - - // Check if error matches an ignored regex - if (this.matchesIgnoredRegex(error)) { - return false; - } - - // Check throttling (for native toast dedup only — storage is unconditional) - if (this.isThrottled(error)) { - return false; - } - - return true; - } - // =========================================================================== // Native Notifications // =========================================================================== /** * Shows a native notification for an error. - * Note: Electron's `subtitle` option only works on macOS. - * On Windows/Linux, we prepend the subtitle to the body instead. + * Closes over `stored` (StoredNotification) so click handler has full data. */ - private showNativeNotification(error: DetectedError): void { - // Guard against standalone/Docker mode where Electron's Notification API is unavailable + private showErrorNativeNotification(stored: StoredNotification): void { + if (!this.isNativeNotificationSupported()) return; + + const config = this.configManager.getConfig(); + const isMac = process.platform === 'darwin'; + const truncatedMessage = stripMarkdown(stored.message).slice(0, 200); + const iconPath = isMac ? undefined : getAppIconPath(); + const notification = new Notification({ + title: 'Claude Code Error', + ...(isMac ? { subtitle: stored.context.projectName } : {}), + body: isMac ? truncatedMessage : `${stored.context.projectName}\n${truncatedMessage}`, + sound: config.notifications.soundEnabled ? 'default' : undefined, + ...(iconPath ? { icon: iconPath } : {}), + }); + + notification.on('click', () => { + this.handleNativeNotificationClick(stored); + }); + + notification.show(); + } + + /** + * Shows a native notification for a team event. + * Uses team-specific formatting (title = team name, subtitle = summary). + */ + private showTeamNativeNotification( + stored: StoredNotification, + payload: TeamNotificationPayload + ): void { + if (!this.isNativeNotificationSupported()) return; + + const config = this.configManager.getConfig(); + const isMac = process.platform === 'darwin'; + const truncatedBody = stripMarkdown(payload.body).slice(0, 300); + const iconPath = isMac ? undefined : getAppIconPath(); + const notification = new Notification({ + title: payload.teamDisplayName, + ...(isMac ? { subtitle: payload.summary } : {}), + body: !isMac && payload.summary ? `${payload.summary}\n${truncatedBody}` : truncatedBody, + sound: config.notifications.soundEnabled ? 'default' : undefined, + ...(iconPath ? { icon: iconPath } : {}), + }); + + notification.on('click', () => { + this.handleNativeNotificationClick(stored); + }); + + notification.show(); + } + + /** + * Shared click handler for native notifications — focuses window and emits deep-link. + */ + private handleNativeNotificationClick(stored: StoredNotification): void { + if (this.mainWindow && !this.mainWindow.isDestroyed()) { + this.mainWindow.show(); + this.mainWindow.focus(); + this.mainWindow.webContents.send('notification:clicked', stored); + } + this.emit('notification-clicked', stored); + } + + /** + * Guard: checks if Electron's Notification API is available. + */ + private isNativeNotificationSupported(): boolean { if ( typeof Notification === 'undefined' || typeof Notification.isSupported !== 'function' || !Notification.isSupported() ) { logger.warn('Native notifications not supported'); - return; + return false; } - - const config = this.configManager.getConfig(); - - const isMac = process.platform === 'darwin'; - const truncatedMessage = stripMarkdown(error.message).slice(0, 200); - const iconPath = isMac ? undefined : getAppIconPath(); - const notification = new Notification({ - title: 'Claude Code Error', - ...(isMac ? { subtitle: error.context.projectName } : {}), - body: isMac ? truncatedMessage : `${error.context.projectName}\n${truncatedMessage}`, - sound: config.notifications.soundEnabled ? 'default' : undefined, - ...(iconPath ? { icon: iconPath } : {}), - }); - - notification.on('click', () => { - // Focus app window - if (this.mainWindow && !this.mainWindow.isDestroyed()) { - this.mainWindow.show(); - this.mainWindow.focus(); - - // Send deep link to renderer - this.mainWindow.webContents.send('notification:clicked', error); - } - - // Emit event for other listeners - this.emit('notification-clicked', error); - }); - - notification.show(); + return true; } // =========================================================================== @@ -463,17 +482,21 @@ export class NotificationManager extends EventEmitter { // =========================================================================== /** - * Adds an error and shows a notification if enabled. - * @param error - The detected error to add - * @returns The stored notification, or null if filtered/throttled + * Stores a notification unconditionally. Emits IPC events to renderer. + * Returns null if dedupeKey already exists in storage (storage-level dedupe) + * or if toolUseId-based dedup skips it. */ - async addError(error: DetectedError): Promise { - // Wait for async initialization to complete before modifying notifications. - // Prevents a race where saveNotifications() overwrites not-yet-loaded data. + private async storeNotification(error: DetectedError): Promise { if (this.initPromise) { await this.initPromise; } + // Storage-level dedupe by dedupeKey (persistent, lives as long as notification is in storage) + if (error.dedupeKey) { + const exists = this.notifications.some((n) => n.dedupeKey === error.dedupeKey); + if (exists) return null; + } + // Deduplicate by toolUseId: the same tool call can appear in both the // subagent JSONL file and the parent session JSONL (as a progress event). // Keep the subagent-annotated version (with subagentId) when possible. @@ -511,12 +534,46 @@ export class NotificationManager extends EventEmitter { // Emit authoritative counters (total/unread) so renderer badge stays in sync. this.emitNotificationUpdated(); - // Show native notification if enabled and not filtered - if (await this.shouldNotify(error)) { - this.showNativeNotification(error); + return storedNotification; + } + + /** + * Adds an error notification. Storage is unconditional; native toast respects + * enabled/snoozed, ignored repos, ignored regex, and 5s throttle. + */ + async addError(error: DetectedError): Promise { + const stored = await this.storeNotification(error); + if (!stored) return null; + + // Error-specific toast policy: repo filter + regex filter + enabled/snoozed + throttle + if ( + this.areNotificationsEnabled() && + !(await this.isFromIgnoredRepository(error)) && + !this.matchesIgnoredRegex(error) && + !this.isToastThrottled(error) + ) { + this.showErrorNativeNotification(stored); } - return storedNotification; + return stored; + } + + /** + * Adds a team notification. Storage is unconditional; native toast respects + * enabled/snoozed, suppressToast flag, and 5s dedupeKey-based throttle. + * Skips repo/regex filters (not applicable to team events). + */ + async addTeamNotification(payload: TeamNotificationPayload): Promise { + const error = buildDetectedErrorFromTeam(payload); + const stored = await this.storeNotification(error); + if (!stored) return null; + + // Team-specific toast policy: enabled/snoozed + suppressToast + dedupeKey throttle only + if (!payload.suppressToast && this.areNotificationsEnabled() && !this.isToastThrottled(error)) { + this.showTeamNativeNotification(stored, payload); + } + + return stored; } /** diff --git a/src/main/utils/teamNotificationBuilder.ts b/src/main/utils/teamNotificationBuilder.ts new file mode 100644 index 00000000..cacff342 --- /dev/null +++ b/src/main/utils/teamNotificationBuilder.ts @@ -0,0 +1,94 @@ +/** + * Team notification builder — creates DetectedError objects from team event payloads. + * + * Pure utility with no service dependencies. Used by NotificationManager.addTeamNotification() + * to convert domain-level team payloads into the unified notification format. + */ + +import { randomUUID } from 'crypto'; + +import type { TriggerColor } from '@shared/constants/triggerColors'; + +import type { DetectedError } from '../services/error/ErrorMessageBuilder'; + +// ============================================================================= +// Types +// ============================================================================= + +export type TeamEventType = + | 'rate_limit' + | 'lead_inbox' + | 'user_inbox' + | 'task_clarification' + | 'task_status_change'; + +/** + * Domain payload for team notifications. + * Single source of truth — both storage and native presentation are derived from this. + */ +export interface TeamNotificationPayload { + teamEventType: TeamEventType; + teamName: string; + teamDisplayName: string; + from: string; + to?: string; + summary: string; + body: string; + /** Stable key for storage deduplication. REQUIRED — no fallback to Date.now(). */ + dedupeKey: string; + projectPath?: string; + /** + * When true, the notification is stored in-app but no native OS toast is shown. + * Used when per-type toggle (e.g. notifyOnLeadInbox) is off — storage is unconditional, + * but the user opted out of OS interruptions for this event type. + */ + suppressToast?: boolean; +} + +// ============================================================================= +// Config mapping +// ============================================================================= + +interface TeamNotificationConfig { + triggerName: string; + triggerColor: TriggerColor; +} + +const TEAM_NOTIFICATION_CONFIG: Record = { + rate_limit: { triggerName: 'Rate Limit', triggerColor: 'red' }, + lead_inbox: { triggerName: 'Team Inbox', triggerColor: 'blue' }, + user_inbox: { triggerName: 'User Inbox', triggerColor: 'green' }, + task_clarification: { triggerName: 'Clarification', triggerColor: 'orange' }, + task_status_change: { triggerName: 'Status Change', triggerColor: 'purple' }, +}; + +// ============================================================================= +// Builder +// ============================================================================= + +/** + * Converts a team notification payload into a DetectedError for unified storage. + * Uses `sessionId: 'team:{teamName}'` convention (established by rate-limit notifications). + */ +export function buildDetectedErrorFromTeam(payload: TeamNotificationPayload): DetectedError { + const config = TEAM_NOTIFICATION_CONFIG[payload.teamEventType]; + + return { + id: randomUUID(), + timestamp: Date.now(), + sessionId: `team:${payload.teamName}`, + projectId: payload.teamName, + filePath: '', + source: payload.teamEventType, + message: `[${payload.from}] ${payload.body.slice(0, 300)}`, + category: 'team', + teamEventType: payload.teamEventType, + dedupeKey: payload.dedupeKey, + triggerColor: config.triggerColor, + triggerName: config.triggerName, + context: { + projectName: payload.teamDisplayName, + cwd: payload.projectPath, + }, + }; +} diff --git a/src/renderer/components/dashboard/CliStatusBanner.tsx b/src/renderer/components/dashboard/CliStatusBanner.tsx index ecdd4e9d..46652a8c 100644 --- a/src/renderer/components/dashboard/CliStatusBanner.tsx +++ b/src/renderer/components/dashboard/CliStatusBanner.tsx @@ -38,6 +38,9 @@ const VARIANT_STYLES: Record = { warning: { border: '#f59e0b', bg: 'rgba(245, 158, 11, 0.06)' }, }; +/** Minimum banner height — prevents layout shift between states (loading → installed → checking). */ +const BANNER_MIN_H = 'min-h-[4.25rem]'; + // ============================================================================= // Sub-components // ============================================================================= @@ -180,7 +183,7 @@ export const CliStatusBanner = (): React.JSX.Element | null => { if (cliStatusError && !cliStatusLoading) { return (
{ if (!cliStatusLoading) { return (
@@ -232,7 +235,7 @@ export const CliStatusBanner = (): React.JSX.Element | null => { // Loading state: show spinner only while an actual request is in-flight. return (
{ if (installerState === 'downloading') { return (
@@ -292,11 +295,11 @@ export const CliStatusBanner = (): React.JSX.Element | null => { installerState === 'checking' ? 'Checking latest version...' : 'Verifying checksum...'; return (
- + {label} @@ -310,11 +313,11 @@ export const CliStatusBanner = (): React.JSX.Element | null => { if (installerState === 'installing') { return (
- + Installing Claude CLI... @@ -328,7 +331,7 @@ export const CliStatusBanner = (): React.JSX.Element | null => { if (installerState === 'completed') { return (
@@ -343,7 +346,7 @@ export const CliStatusBanner = (): React.JSX.Element | null => { if (installerState === 'error') { return (
@@ -446,7 +449,7 @@ export const CliStatusBanner = (): React.JSX.Element | null => { // Installed — show version, path, update info return (
diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 950d96f1..c5e8285e 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -7,7 +7,7 @@ import { useState } from 'react'; import { getTriggerColorDef } from '@shared/constants/triggerColors'; import { formatDistanceToNow } from 'date-fns'; -import { ArrowRight, Bot, Check, Trash2 } from 'lucide-react'; +import { ArrowRight, Bot, Check, Trash2, Users } from 'lucide-react'; import type { DetectedError } from '@renderer/types/data'; @@ -41,6 +41,7 @@ export const NotificationRow = ({ const truncatedMessage = truncateMessage(error.message); const colorDef = getTriggerColorDef(error.triggerColor); const displayName = error.triggerName ?? error.source; + const isTeamNotification = error.category === 'team' || error.sessionId?.startsWith('team:'); const handleArchiveClick = (e: React.MouseEvent): void => { e.stopPropagation(); @@ -102,6 +103,19 @@ export const NotificationRow = ({ {projectName} + {isTeamNotification && !error.subagentId && ( + + + team + + )} {error.subagentId && ( )} - {showMoreVisible && ( - - )}
@@ -527,6 +516,21 @@ export const ClaudeLogsSection = ({ teamName }: ClaudeLogsSectionProps): React.J void loadOlderLogs(); } }} + footer={ + showMoreVisible ? ( +
+ +
+ ) : null + } /> ) : null} {!error && data.lines.length === 0 ? ( diff --git a/src/renderer/components/team/CliLogsRichView.tsx b/src/renderer/components/team/CliLogsRichView.tsx index 1cfa5805..c5ccf6be 100644 --- a/src/renderer/components/team/CliLogsRichView.tsx +++ b/src/renderer/components/team/CliLogsRichView.tsx @@ -31,6 +31,8 @@ interface CliLogsRichViewProps { /** Optional local search query override for inline highlighting */ searchQueryOverride?: string; className?: string; + /** Content rendered at the very bottom of the scroll container (e.g. "Show more" button). */ + footer?: React.ReactNode; } /** @@ -237,6 +239,7 @@ export const CliLogsRichView = ({ containerRefCallback, searchQueryOverride, className, + footer, }: CliLogsRichViewProps): React.JSX.Element => { const scrollRef = useRef(null); const stickToEdgeRef = useRef(true); @@ -370,6 +373,7 @@ export const CliLogsRichView = ({ Waiting for CLI output...

)} + {footer}
); } @@ -426,6 +430,7 @@ export const CliLogsRichView = ({ /> ) )} + {footer}
); }; diff --git a/src/renderer/components/team/MemberBadge.tsx b/src/renderer/components/team/MemberBadge.tsx index a6857a06..ff431671 100644 --- a/src/renderer/components/team/MemberBadge.tsx +++ b/src/renderer/components/team/MemberBadge.tsx @@ -1,4 +1,9 @@ -import { getTeamColorSet, getThemedBadge } from '@renderer/constants/teamColors'; +import { + getTeamColorSet, + getThemedBadge, + getThemedBorder, + getThemedText, +} from '@renderer/constants/teamColors'; import { useTheme } from '@renderer/hooks/useTheme'; import { agentAvatarUrl } from '@renderer/utils/memberHelpers'; @@ -32,8 +37,8 @@ export const MemberBadge = ({ const badgeStyle = { backgroundColor: getThemedBadge(colors, isLight), - color: colors.text, - border: `1px solid ${colors.border}40`, + color: getThemedText(colors, isLight), + border: `1px solid ${getThemedBorder(colors, isLight)}40`, }; const avatar = ( diff --git a/src/renderer/components/team/activity/ActivityItem.tsx b/src/renderer/components/team/activity/ActivityItem.tsx index 1ba1384d..d46c8106 100644 --- a/src/renderer/components/team/activity/ActivityItem.tsx +++ b/src/renderer/components/team/activity/ActivityItem.tsx @@ -13,7 +13,8 @@ import { CARD_ICON_MUTED, CARD_TEXT_LIGHT, } from '@renderer/constants/cssVariables'; -import { getTeamColorSet } from '@renderer/constants/teamColors'; +import { getTeamColorSet, getThemedBorder } from '@renderer/constants/teamColors'; +import { useTheme } from '@renderer/hooks/useTheme'; import { getMessageTypeLabel, getStructuredMessageSummary, @@ -250,6 +251,7 @@ export const ActivityItem = ({ collapseState, }: ActivityItemProps): React.JSX.Element => { const colors = getTeamColorSet(memberColor ?? message.color ?? ''); + const { isLight } = useTheme(); const formattedRole = formatAgentRole(memberRole); const timestamp = Number.isNaN(Date.parse(message.timestamp)) @@ -353,7 +355,7 @@ export const ActivityItem = ({ ? '3px solid var(--tool-result-error-text)' : isSystemMessage ? '3px solid var(--system-activity-accent)' - : `3px solid ${colors.border}`, + : `3px solid ${getThemedBorder(colors, isLight)}`, }} > {/* Header — div with role=button (cannot use
-
+
+ {onReply ? ( + + + + + Reply + + ) : null}
@@ -393,6 +427,7 @@ export const LeadThoughtsGroupRow = ({ collapseState, onTaskIdClick, memberColorMap, + onReply, }: LeadThoughtsGroupRowProps): React.JSX.Element => { const ref = useRef(null); const scrollRef = useRef(null); @@ -559,7 +594,7 @@ export const LeadThoughtsGroupRow = ({ [clearPendingScrollSync, expanded, isBodyVisible, queueScrollSync] ); - useEffect(() => { + useLayoutEffect(() => { if (!isBodyVisible) return; const contentEl = contentRef.current; if (!contentEl) return; @@ -612,11 +647,7 @@ export const LeadThoughtsGroupRow = ({ }, []); return ( -
+
) : null} - {/* Live / offline indicator */} - {isLive ? ( - - - - - ) : ( - - )} + {/* Lead avatar with optional live indicator */} +
+ + {isLive ? ( + + + + + ) : null} +
{thoughts.length} thoughts @@ -716,6 +753,7 @@ export const LeadThoughtsGroupRow = ({ shouldAnimate={isLive && idx === chronologicalThoughts.length - 1} onTaskIdClick={onTaskIdClick} memberColorMap={memberColorMap} + onReply={onReply} /> ))}
@@ -758,6 +796,6 @@ export const LeadThoughtsGroupRow = ({
) : null} -
+ ); }; diff --git a/src/renderer/components/team/attachments/AttachmentPreviewList.tsx b/src/renderer/components/team/attachments/AttachmentPreviewList.tsx index 42c0043f..91fd1f98 100644 --- a/src/renderer/components/team/attachments/AttachmentPreviewList.tsx +++ b/src/renderer/components/team/attachments/AttachmentPreviewList.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { AlertCircle, X } from 'lucide-react'; @@ -7,6 +7,8 @@ import { ImageLightbox } from './ImageLightbox'; import type { AttachmentPayload } from '@shared/types'; +const ANIMATION_MS = 400; + interface AttachmentPreviewListProps { attachments: AttachmentPayload[]; onRemove: (id: string) => void; @@ -27,30 +29,117 @@ export const AttachmentPreviewList = ({ disabledHint, }: AttachmentPreviewListProps): React.JSX.Element | null => { const [lightboxIndex, setLightboxIndex] = useState(null); + const [exitingIds, setExitingIds] = useState>(new Set()); + // Track IDs known on previous render to detect newly added items + const knownIdsRef = useRef>(new Set()); + const [enteringIds, setEnteringIds] = useState>(new Set()); + const exitTimersRef = useRef>(new Map()); + const enterTimersRef = useRef>(new Map()); - if (attachments.length === 0 && !error) return null; + // Detect newly added attachments + useEffect(() => { + const currentIds = new Set(attachments.map((a) => a.id)); + const newIds = new Set(); + for (const id of currentIds) { + if (!knownIdsRef.current.has(id)) { + newIds.add(id); + } + } + knownIdsRef.current = currentIds; - const lightboxSlides = attachments.map((att) => ({ + if (newIds.size === 0) return; + + setEnteringIds((prev) => { + const next = new Set(prev); + for (const id of newIds) next.add(id); + return next; + }); + + // Clear entering state after animation completes + for (const id of newIds) { + const timer = window.setTimeout(() => { + setEnteringIds((prev) => { + const next = new Set(prev); + next.delete(id); + return next; + }); + enterTimersRef.current.delete(id); + }, ANIMATION_MS); + enterTimersRef.current.set(id, timer); + } + }, [attachments]); + + // Cleanup timers on unmount + useEffect(() => { + return () => { + for (const t of exitTimersRef.current.values()) window.clearTimeout(t); + for (const t of enterTimersRef.current.values()) window.clearTimeout(t); + }; + }, []); + + const handleRemove = useCallback( + (id: string) => { + // Start exit animation + setExitingIds((prev) => new Set(prev).add(id)); + + // Actually remove after animation + const timer = window.setTimeout(() => { + setExitingIds((prev) => { + const next = new Set(prev); + next.delete(id); + return next; + }); + exitTimersRef.current.delete(id); + onRemove(id); + }, ANIMATION_MS); + exitTimersRef.current.set(id, timer); + }, + [onRemove] + ); + + // Include exiting items that are no longer in attachments (they were removed by parent) + // This shouldn't normally happen since we delay onRemove, but guard against it. + const visibleAttachments = attachments; + + if (visibleAttachments.length === 0 && exitingIds.size === 0 && !error) return null; + + const lightboxSlides = visibleAttachments.map((att) => ({ src: `data:${att.mimeType};base64,${att.data}`, alt: att.filename, })); return (
- {attachments.length > 0 ? ( + {visibleAttachments.length > 0 ? (
- {attachments.map((att, i) => ( - setLightboxIndex(i)} - disabled={disabled} - /> - ))} + {visibleAttachments.map((att, i) => { + const isExiting = exitingIds.has(att.id); + const isEntering = enteringIds.has(att.id); + return ( +
+ setLightboxIndex(i)} + disabled={disabled} + /> +
+ ); + })}
) : null} - {disabled && disabledHint && attachments.length > 0 ? ( + {disabled && disabledHint && visibleAttachments.length > 0 ? (
{quote ? ( -
+
{/* Decorative quotation mark */} - + “ @@ -341,7 +341,7 @@ export const SendMessageDialog = ({ - - Reply to comment - - - - -
- {(() => { - const reply = parseMessageReply(comment.text); - const rawForDisplay = reply ? reply.replyText : comment.text; - const displayText = normalizeLiteralNewlines(stripAgentBlocks(rawForDisplay)); - return ( - - {reply ? ( - + + + + Reply to comment + + + + +
+ {(() => { + const reply = parseMessageReply(comment.text); + const rawForDisplay = reply ? reply.replyText : comment.text; + const displayText = normalizeLiteralNewlines(stripAgentBlocks(rawForDisplay)); + return ( + + {reply ? ( + - - )} - - ); - })()} - {comment.attachments && comment.attachments.length > 0 ? ( - - ) : null} -
+ ) : ( + { + const link = ( + e.target as HTMLElement + ).closest('a[href^="task://"]'); + if (link) { + e.preventDefault(); + e.stopPropagation(); + const id = link.getAttribute('href')?.replace('task://', ''); + if (id) onTaskIdClick(id); + } + } + : undefined + } + > + { + let t = linkifyTaskIdsInMarkdown(displayText); + if (colorMap.size > 0) t = linkifyMentionsInMarkdown(t, colorMap); + return t; + })()} + maxHeight="max-h-none" + bare + /> + + )} + + ); + })()} + {comment.attachments && comment.attachments.length > 0 ? ( + + ) : null} +
+ ))}
diff --git a/src/renderer/components/ui/MentionableTextarea.tsx b/src/renderer/components/ui/MentionableTextarea.tsx index 63c88811..956fe3f8 100644 --- a/src/renderer/components/ui/MentionableTextarea.tsx +++ b/src/renderer/components/ui/MentionableTextarea.tsx @@ -601,7 +601,7 @@ export const MentionableTextarea = React.forwardRef [ 'Tip: Use @ to mention team members or search files', - 'Tip: Mention "create a task" to add it to the kanban', + 'Tip: Mention "delegate a task to a teammate" to add it to the kanban', "Tip: Don't overload the team lead with tasks — ask them to delegate to teammates", ], [] diff --git a/src/renderer/constants/teamColors.ts b/src/renderer/constants/teamColors.ts index eb345fb3..c57ecc4d 100644 --- a/src/renderer/constants/teamColors.ts +++ b/src/renderer/constants/teamColors.ts @@ -8,6 +8,8 @@ export interface TeamColorSet { /** Border accent color */ border: string; + /** Border accent color for light theme */ + borderLight?: string; /** Badge background (semi-transparent) */ badge: string; /** Badge background for light theme (more visible on white) */ @@ -85,10 +87,11 @@ const TEAMMATE_COLORS: Record = { /** Reserved for the human user — never assigned to team members. */ user: { border: '#f5f5f4', + borderLight: '#a8a29e', badge: 'rgba(245, 245, 244, 0.12)', - badgeLight: 'rgba(0, 0, 0, 0.08)', + badgeLight: 'rgba(120, 113, 108, 0.14)', text: '#d6d3d1', - textLight: '#57534e', + textLight: '#44403c', }, }; @@ -148,3 +151,17 @@ export function getTeamColorSet(colorName: string): TeamColorSet { export function getThemedBadge(colorSet: TeamColorSet, isLight: boolean): string { return isLight && colorSet.badgeLight ? colorSet.badgeLight : colorSet.badge; } + +/** + * Get the appropriate text color for the current theme. + */ +export function getThemedText(colorSet: TeamColorSet, isLight: boolean): string { + return isLight && colorSet.textLight ? colorSet.textLight : colorSet.text; +} + +/** + * Get the appropriate border color for the current theme. + */ +export function getThemedBorder(colorSet: TeamColorSet, isLight: boolean): string { + return isLight && colorSet.borderLight ? colorSet.borderLight : colorSet.border; +} diff --git a/src/renderer/hooks/useTheme.ts b/src/renderer/hooks/useTheme.ts index dc6bc9f3..19b689c8 100644 --- a/src/renderer/hooks/useTheme.ts +++ b/src/renderer/hooks/useTheme.ts @@ -32,11 +32,12 @@ export function useTheme(): { // Initialize from cache to prevent flash try { const cached = localStorage.getItem(THEME_CACHE_KEY); - if (cached === 'light') return 'light'; + if (cached === 'light' || cached === 'dark') return cached; } catch { // localStorage may not be available } - return 'dark'; + // No cache — detect system preference for flash-free first launch + return window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'; }); // Fetch config on mount if not loaded. @@ -50,7 +51,7 @@ export function useTheme(): { }, [appConfig, configLoading, fetchConfig]); // Get configured theme - const configuredTheme: Theme = appConfig?.general?.theme ?? 'dark'; + const configuredTheme: Theme = appConfig?.general?.theme ?? 'system'; // Get system theme preference const getSystemTheme = useCallback((): ResolvedTheme => { diff --git a/src/renderer/index.css b/src/renderer/index.css index 467ac62c..01a4a51b 100644 --- a/src/renderer/index.css +++ b/src/renderer/index.css @@ -624,21 +624,6 @@ body { } } -@keyframes message-enter { - from { - opacity: 0; - transform: translateY(-8px); - } - to { - opacity: 1; - transform: translateY(0); - } -} - -.message-enter-animate { - animation: message-enter 300ms ease-out both; -} - @keyframes chat-message-enter { from { opacity: 0; @@ -670,6 +655,17 @@ body { animation: thought-expand 350ms ease-out both; } +@keyframes att-scale-in { + from { + transform: scale(0); + opacity: 0; + } + to { + transform: scale(1); + opacity: 1; + } +} + .skeleton-card { animation: skeleton-fade-in 0.4s ease-out both; position: relative; diff --git a/src/renderer/store/slices/notificationSlice.ts b/src/renderer/store/slices/notificationSlice.ts index d9fc16df..2d0d3f60 100644 --- a/src/renderer/store/slices/notificationSlice.ts +++ b/src/renderer/store/slices/notificationSlice.ts @@ -206,8 +206,8 @@ export const createNotificationSlice: StateCreator t.teamName === task.teamName && t.id === task.id); if (oldTask?.needsClarification !== 'user' && !notifiedClarificationTaskKeys.has(key)) { notifiedClarificationTaskKeys.add(key); - if (notifyEnabled) { - fireClarificationNotification(task); - } + // Always store in-app; suppress OS toast when per-type toggle is off + fireClarificationNotification(task, !notifyEnabled); } } else { notifiedClarificationTaskKeys.delete(key); @@ -116,18 +115,22 @@ function detectClarificationNotifications( } } -function fireClarificationNotification(task: GlobalTask): void { +function fireClarificationNotification(task: GlobalTask, suppressToast: boolean): void { // Delegate to main process for native OS notification (cross-platform, no permission needed) const latestComment = task.comments?.length ? task.comments[task.comments.length - 1] : undefined; const body = latestComment?.text || task.description || `Task #${task.id}: ${task.subject}`; void api.teams ?.showMessageNotification({ + teamName: task.teamName, teamDisplayName: task.teamDisplayName, from: latestComment?.author || 'team-lead', to: 'user', summary: `Clarification needed — Task #${task.id}`, body, + teamEventType: 'task_clarification', + dedupeKey: `clarification:${task.teamName}:${task.id}:${task.updatedAt ?? Date.now()}`, + suppressToast, }) .catch(() => undefined); } @@ -138,13 +141,12 @@ function detectStatusChangeNotifications( config: AppConfig | null, teamByName: Record ): void { - if (!config?.notifications?.notifyOnStatusChange) return; - if (!config.notifications.enabled) return; - - const statuses = config.notifications.statusChangeStatuses ?? ['in_progress', 'completed']; + const statusChangeEnabled = + !!config?.notifications?.notifyOnStatusChange && !!config.notifications.enabled; + const statuses = config?.notifications?.statusChangeStatuses ?? ['in_progress', 'completed']; if (statuses.length === 0) return; - const onlySolo = config.notifications.statusChangeOnlySolo ?? true; + const onlySolo = config?.notifications?.statusChangeOnlySolo ?? true; for (const task of newTasks) { const oldTask = oldTasks.find((t) => t.teamName === task.teamName && t.id === task.id); @@ -170,14 +172,20 @@ function detectStatusChangeNotifications( notifiedStatusChangeKeys.add(key); const fromLabel = becameApproved ? 'Completed' : oldTask.status; - fireStatusChangeNotification(task, fromLabel, becameApproved ? 'approved' : undefined); + fireStatusChangeNotification( + task, + fromLabel, + becameApproved ? 'approved' : undefined, + !statusChangeEnabled + ); } } function fireStatusChangeNotification( task: GlobalTask, fromStatus: string, - overrideToStatus?: string + overrideToStatus?: string, + suppressToast?: boolean ): void { const statusLabels: Record = { pending: 'Pending', @@ -192,11 +200,15 @@ function fireStatusChangeNotification( void api.teams ?.showMessageNotification({ + teamName: task.teamName, teamDisplayName: task.teamDisplayName, from: task.owner ?? 'system', to: 'user', summary: `Task #${task.id}: ${from} → ${to}`, body: task.subject, + teamEventType: 'task_status_change', + dedupeKey: `status:${task.teamName}:${task.id}:${fromStatus}:${toStatus}:${task.updatedAt ?? Date.now()}`, + suppressToast, }) .catch(() => undefined); } diff --git a/src/shared/types/notifications.ts b/src/shared/types/notifications.ts index 030aa63f..e86acaeb 100644 --- a/src/shared/types/notifications.ts +++ b/src/shared/types/notifications.ts @@ -50,6 +50,17 @@ export interface DetectedError { triggerId?: string; /** Human-readable name of the trigger that produced this notification */ triggerName?: string; + /** Notification domain: 'error' (default/undefined) or 'team' */ + category?: 'error' | 'team'; + /** For team notifications: specific event sub-type */ + teamEventType?: + | 'rate_limit' + | 'lead_inbox' + | 'user_inbox' + | 'task_clarification' + | 'task_status_change'; + /** Explicit key for storage deduplication. Two notifications with the same dedupeKey won't be stored twice. */ + dedupeKey?: string; /** Additional context */ context: { /** Display name of the project */ diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index 5ce1986e..84658599 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -516,6 +516,8 @@ export interface ReplaceMembersRequest { /** Data sent from renderer to main for native OS team message notification. */ export interface TeamMessageNotificationData { teamDisplayName: string; + /** Team directory name (for notification storage and deep-linking). */ + teamName?: string; /** Who sent the message. */ from: string; /** Who received the message (member name or "user"). */ @@ -526,6 +528,16 @@ export interface TeamMessageNotificationData { body: string; /** Optional sender color for visual context. */ color?: string; + /** Team event sub-type for notification categorization. */ + teamEventType?: 'task_clarification' | 'task_status_change'; + /** Stable key for storage deduplication. Required — no fallback to Date.now(). */ + dedupeKey?: string; + /** + * When true, the notification is stored in-app but no native OS toast is shown. + * Used when per-type toggle is off — storage is unconditional, + * but the user opted out of OS interruptions for this event type. + */ + suppressToast?: boolean; } // ============================================================================= diff --git a/test/main/ipc/teams.test.ts b/test/main/ipc/teams.test.ts index 959bc7cf..dcbe0f61 100644 --- a/test/main/ipc/teams.test.ts +++ b/test/main/ipc/teams.test.ts @@ -14,6 +14,18 @@ vi.mock('@preload/constants/ipcChannels', async (importOriginal) => { return { ...actual }; }); +// Mock NotificationManager — handleShowMessageNotification calls addTeamNotification +const { mockAddTeamNotification } = vi.hoisted(() => ({ + mockAddTeamNotification: vi.fn().mockResolvedValue({ id: 'n1', isRead: false, createdAt: Date.now() }), +})); +vi.mock('@main/services/infrastructure/NotificationManager', () => ({ + NotificationManager: { + getInstance: vi.fn().mockReturnValue({ + addTeamNotification: mockAddTeamNotification, + }), + }, +})); + import { TEAM_ALIVE_LIST, TEAM_STOP, @@ -668,6 +680,61 @@ describe('ipc teams handlers', () => { }); }); + describe('showMessageNotification', () => { + it('returns success on valid notification data', async () => { + const handler = handlers.get(TEAM_SHOW_MESSAGE_NOTIFICATION)!; + const result = (await handler({} as never, { + teamDisplayName: 'My Team', + from: 'alice', + body: 'Hello!', + teamName: 'my-team', + teamEventType: 'task_clarification', + dedupeKey: 'clarification:my-team:42', + })) as { success: boolean }; + expect(result.success).toBe(true); + }); + + it('rejects when missing required fields', async () => { + const handler = handlers.get(TEAM_SHOW_MESSAGE_NOTIFICATION)!; + const result = (await handler({} as never, { + teamDisplayName: 'My Team', + // missing from and body + })) as { success: boolean; error: string }; + expect(result.success).toBe(false); + expect(result.error).toContain('Missing required fields'); + }); + + it('rejects null data', async () => { + const handler = handlers.get(TEAM_SHOW_MESSAGE_NOTIFICATION)!; + const result = (await handler({} as never, null)) as { success: boolean }; + expect(result.success).toBe(false); + }); + + it('generates fallback dedupeKey when not provided', async () => { + const handler = handlers.get(TEAM_SHOW_MESSAGE_NOTIFICATION)!; + const result = (await handler({} as never, { + teamDisplayName: 'My Team', + teamName: 'my-team', + from: 'bob', + body: 'Some message', + })) as { success: boolean }; + // Should succeed even without explicit dedupeKey (fallback is generated) + expect(result.success).toBe(true); + }); + + it('rejects when teamName is missing', async () => { + const handler = handlers.get(TEAM_SHOW_MESSAGE_NOTIFICATION)!; + const result = (await handler({} as never, { + teamDisplayName: 'My Team', + from: 'alice', + body: 'Hello!', + // teamName intentionally omitted + })) as { success: boolean; error: string }; + expect(result.success).toBe(false); + expect(result.error).toContain('teamName'); + }); + }); + describe('reserved teammate names', () => { it('rejects teammate name "user" in createTeam', async () => { const handler = handlers.get(TEAM_CREATE)!; diff --git a/test/main/services/infrastructure/NotificationManager.team.test.ts b/test/main/services/infrastructure/NotificationManager.team.test.ts new file mode 100644 index 00000000..5b5236ae --- /dev/null +++ b/test/main/services/infrastructure/NotificationManager.team.test.ts @@ -0,0 +1,259 @@ +/** + * NotificationManager team notification tests. + * + * Tests the addTeamNotification() adapter and its interaction with the + * shared storage pipeline (storeNotification), dedupeKey-based dedupe, + * and toast throttling. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { TeamNotificationPayload } from '@main/utils/teamNotificationBuilder'; + +// --- Mock electron Notification before importing NotificationManager --- +const mockNotificationShow = vi.fn(); +const mockNotificationOn = vi.fn(); +vi.mock('electron', () => ({ + Notification: Object.assign( + vi.fn().mockImplementation(() => ({ + show: mockNotificationShow, + on: mockNotificationOn, + })), + { isSupported: vi.fn().mockReturnValue(true) } + ), + BrowserWindow: vi.fn(), +})); + +// --- Mock fs/promises to prevent disk I/O --- +vi.mock('fs/promises', () => ({ + readFile: vi.fn().mockRejectedValue({ code: 'ENOENT' }), + writeFile: vi.fn().mockResolvedValue(undefined), + mkdir: vi.fn().mockResolvedValue(undefined), +})); + +// --- Mock ConfigManager --- +vi.mock('@main/services/infrastructure/ConfigManager', () => ({ + ConfigManager: { + getInstance: vi.fn().mockReturnValue({ + getConfig: vi.fn().mockReturnValue({ + notifications: { + enabled: true, + soundEnabled: false, + snoozedUntil: null, + ignoredRegex: [], + ignoredRepositories: [], + }, + }), + clearSnooze: vi.fn(), + }), + }, +})); + +// --- Mock path/service dependencies that NotificationManager imports --- +vi.mock('@main/services/discovery/ProjectPathResolver', () => ({ + projectPathResolver: { resolveProjectPath: vi.fn().mockResolvedValue('/tmp') }, +})); +vi.mock('@main/services/parsing/GitIdentityResolver', () => ({ + gitIdentityResolver: { resolveIdentity: vi.fn().mockResolvedValue(null) }, +})); +vi.mock('@main/utils/appIcon', () => ({ + getAppIconPath: vi.fn().mockReturnValue(undefined), +})); +vi.mock('@main/utils/textFormatting', () => ({ + stripMarkdown: vi.fn((s: string) => s), +})); + +import { ConfigManager } from '@main/services/infrastructure/ConfigManager'; +import { NotificationManager } from '@main/services/infrastructure/NotificationManager'; + +function makeTeamPayload( + overrides: Partial = {} +): TeamNotificationPayload { + return { + teamEventType: 'user_inbox', + teamName: 'test-team', + teamDisplayName: 'Test Team', + from: 'alice', + summary: 'New message from Alice', + body: 'Hello from Alice!', + dedupeKey: `inbox:test-team:alice:${Date.now()}`, + ...overrides, + }; +} + +describe('NotificationManager.addTeamNotification', () => { + let manager: NotificationManager; + + const defaultConfig = { + notifications: { + enabled: true, + soundEnabled: false, + snoozedUntil: null, + ignoredRegex: [], + ignoredRepositories: [], + }, + }; + + beforeEach(async () => { + NotificationManager.resetInstance(); + manager = new NotificationManager(); + await manager.initialize(); + mockNotificationShow.mockClear(); + mockNotificationOn.mockClear(); + // Restore default config — tests that override must not leak state + const configMock = ConfigManager.getInstance().getConfig as ReturnType; + configMock.mockReturnValue(defaultConfig); + }); + + afterEach(() => { + NotificationManager.resetInstance(); + }); + + it('stores team notification and returns StoredNotification', async () => { + const result = await manager.addTeamNotification(makeTeamPayload()); + + expect(result).not.toBeNull(); + expect(result!.category).toBe('team'); + expect(result!.teamEventType).toBe('user_inbox'); + expect(result!.isRead).toBe(false); + expect(result!.createdAt).toBeGreaterThan(0); + expect(result!.sessionId).toBe('team:test-team'); + expect(result!.dedupeKey).toContain('inbox:test-team:alice:'); + }); + + it('shows native toast when notifications are enabled', async () => { + await manager.addTeamNotification(makeTeamPayload()); + expect(mockNotificationShow).toHaveBeenCalledOnce(); + }); + + it('stores notification but suppresses toast when suppressToast is true', async () => { + const result = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'suppress-test' }), + ); + // Clear from the first call + mockNotificationShow.mockClear(); + + const result2 = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'suppress-test-2', suppressToast: true }), + ); + + expect(result).not.toBeNull(); + expect(result2).not.toBeNull(); + // The second call with suppressToast=true should NOT show a toast + expect(mockNotificationShow).not.toHaveBeenCalled(); + }); + + it('stores notification even when notifications are disabled (storage is unconditional)', async () => { + const configMock = ConfigManager.getInstance().getConfig as ReturnType; + configMock.mockReturnValue({ + notifications: { + enabled: false, + soundEnabled: false, + snoozedUntil: null, + ignoredRegex: [], + ignoredRepositories: [], + }, + }); + + const result = await manager.addTeamNotification(makeTeamPayload()); + + expect(result).not.toBeNull(); + expect(result!.category).toBe('team'); + // But no native toast + expect(mockNotificationShow).not.toHaveBeenCalled(); + }); + + it('stores notification even when snoozed (storage is unconditional)', async () => { + const configMock = ConfigManager.getInstance().getConfig as ReturnType; + configMock.mockReturnValue({ + notifications: { + enabled: true, + soundEnabled: false, + snoozedUntil: Date.now() + 60_000, // snoozed for 1 minute + ignoredRegex: [], + ignoredRepositories: [], + }, + }); + + const result = await manager.addTeamNotification(makeTeamPayload()); + + expect(result).not.toBeNull(); + expect(mockNotificationShow).not.toHaveBeenCalled(); + }); + + it('deduplicates by dedupeKey — same key returns null on second call', async () => { + const payload = makeTeamPayload({ dedupeKey: 'unique-key-123' }); + + const first = await manager.addTeamNotification(payload); + const second = await manager.addTeamNotification(payload); + + expect(first).not.toBeNull(); + expect(second).toBeNull(); + }); + + it('does not deduplicate different dedupeKeys', async () => { + const first = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'key-1' }) + ); + const second = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'key-2' }) + ); + + expect(first).not.toBeNull(); + expect(second).not.toBeNull(); + }); + + it('throttles native toast for same dedupeKey within 5s', async () => { + // First call with a unique dedupeKey (not in storage yet) — shows toast + const result1 = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'throttle-key-a' }) + ); + expect(result1).not.toBeNull(); + + // Second call with different dedupeKey — also shows toast (different key) + const result2 = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'throttle-key-b' }) + ); + expect(result2).not.toBeNull(); + + // Both should have shown toasts (different keys, not throttled) + expect(mockNotificationShow).toHaveBeenCalledTimes(2); + }); + + it('is accessible via getNotifications', async () => { + await manager.addTeamNotification(makeTeamPayload({ dedupeKey: 'get-test' })); + const result = await manager.getNotifications({ limit: 10 }); + + expect(result.notifications).toHaveLength(1); + expect(result.notifications[0].category).toBe('team'); + expect(result.unreadCount).toBe(1); + }); + + it('increments unread count correctly', async () => { + await manager.addTeamNotification(makeTeamPayload({ dedupeKey: 'count-1' })); + await manager.addTeamNotification(makeTeamPayload({ dedupeKey: 'count-2' })); + + expect(manager.getUnreadCountSync()).toBe(2); + }); + + it('markRead works on team notifications', async () => { + const stored = await manager.addTeamNotification(makeTeamPayload({ dedupeKey: 'read-test' })); + expect(stored).not.toBeNull(); + + await manager.markRead(stored!.id); + expect(manager.getUnreadCountSync()).toBe(0); + }); + + it('deleteNotification removes team notification', async () => { + const stored = await manager.addTeamNotification( + makeTeamPayload({ dedupeKey: 'delete-test' }) + ); + expect(stored).not.toBeNull(); + + const deleted = manager.deleteNotification(stored!.id); + expect(deleted).toBe(true); + + const result = await manager.getNotifications({ limit: 10 }); + expect(result.notifications).toHaveLength(0); + }); +}); diff --git a/test/main/services/team/teamctl.test.ts b/test/main/services/team/teamctl.test.ts index 01c3b083..7b3b0ee5 100644 --- a/test/main/services/team/teamctl.test.ts +++ b/test/main/services/team/teamctl.test.ts @@ -295,6 +295,37 @@ describe('teamctl.js', () => { expect(parsed.owner).toBe('bob'); }); + it('creates blocked task with reverse links and keeps status pending even with owner', () => { + writeTask(claudeDir, '1', { id: '1', subject: 'API contract', status: 'pending' }); + writeTask(claudeDir, '2', { id: '2', subject: 'Database schema', status: 'pending' }); + writeTask(claudeDir, '3', { id: '3', subject: 'Frontend shell', status: 'pending' }); + + const { stdout, exitCode } = run(claudeDir, [ + 'task', + 'create', + '--subject', + 'Implement feature', + '--owner', + 'bob', + '--blocked-by', + '1,2', + '--related', + '3', + ]); + + expect(exitCode).toBe(0); + const parsed = JSON.parse(stdout); + expect(parsed.id).toBe('4'); + expect(parsed.owner).toBe('bob'); + expect(parsed.status).toBe('pending'); + expect(parsed.blockedBy).toEqual(['1', '2']); + expect(parsed.related).toEqual(['3']); + + expect(readTask(claudeDir, '1').blocks).toEqual(['4']); + expect(readTask(claudeDir, '2').blocks).toEqual(['4']); + expect(readTask(claudeDir, '3').related).toEqual(['4']); + }); + it('increments task IDs', () => { run(claudeDir, ['task', 'create', '--subject', 'Task 1']); run(claudeDir, ['task', 'create', '--subject', 'Task 2']); @@ -356,8 +387,9 @@ describe('teamctl.js', () => { expect(inbox.length).toBe(1); const msg = inbox[0] as Record; expect(msg.from).toBe('alice'); - expect(String(msg.text)).toContain('New task assigned'); - expect(String(msg.text)).toContain('#1'); + expect(String(msg.text)).toMatch(/^New task assigned to you: #1 "Assigned task"\./); + expect(msg.summary).toBe('New task #1 assigned'); + expect(msg.source).toBe('system_notification'); }); it('sends inbox notification with --notify including prompt and tool instructions', () => { @@ -563,6 +595,63 @@ describe('teamctl.js', () => { }); }); + // ========================================================================= + // Task Link / Unlink + // ========================================================================= + describe('task link / unlink', () => { + beforeEach(() => { + writeTask(claudeDir, '1', { id: '1', subject: 'Foundation', status: 'pending' }); + writeTask(claudeDir, '2', { id: '2', subject: 'Feature', status: 'pending' }); + writeTask(claudeDir, '3', { id: '3', subject: 'Docs', status: 'pending' }); + }); + + it('task link --blocked-by updates reverse blocks relationship', () => { + const { stdout, exitCode } = run(claudeDir, ['task', 'link', '2', '--blocked-by', '1']); + + expect(exitCode).toBe(0); + expect(stdout).toBe('OK task #2 blocked-by #1\n'); + expect(readTask(claudeDir, '2').blockedBy).toEqual(['1']); + expect(readTask(claudeDir, '1').blocks).toEqual(['2']); + }); + + it('task link --blocks delegates to reverse blockedBy relationship', () => { + const { stdout, exitCode } = run(claudeDir, ['task', 'link', '1', '--blocks', '2']); + + expect(exitCode).toBe(0); + expect(stdout).toBe('OK task #1 blocks #2\n'); + expect(readTask(claudeDir, '1').blocks).toEqual(['2']); + expect(readTask(claudeDir, '2').blockedBy).toEqual(['1']); + }); + + it('task unlink removes related links symmetrically', () => { + expect(run(claudeDir, ['task', 'link', '2', '--related', '3']).exitCode).toBe(0); + expect(readTask(claudeDir, '2').related).toEqual(['3']); + expect(readTask(claudeDir, '3').related).toEqual(['2']); + + const { stdout, exitCode } = run(claudeDir, ['task', 'unlink', '2', '--related', '3']); + + expect(exitCode).toBe(0); + expect(stdout).toBe('OK task #2 unlinked related #3\n'); + expect(readTask(claudeDir, '2').related).toEqual([]); + expect(readTask(claudeDir, '3').related).toEqual([]); + }); + + it('fails when link type is ambiguous', () => { + const { exitCode, stderr } = run(claudeDir, [ + 'task', + 'link', + '2', + '--blocked-by', + '1', + '--related', + '3', + ]); + + expect(exitCode).not.toBe(0); + expect(stderr).toContain('Specify exactly one'); + }); + }); + // ========================================================================= // Task Set-Owner / Assign // ========================================================================= @@ -652,8 +741,9 @@ describe('teamctl.js', () => { expect(inbox.length).toBe(1); const msg = inbox[0] as Record; expect(msg.from).toBe('alice'); - expect(String(msg.text)).toContain('Task assigned to you'); - expect(String(msg.text)).toContain('#1'); + expect(String(msg.text)).toMatch(/^Task assigned to you: #1 "Unowned task"\./); + expect(msg.summary).toBe('Task #1 assigned'); + expect(msg.source).toBe('system_notification'); }); it('does NOT send notification without --notify', () => { @@ -749,6 +839,10 @@ describe('teamctl.js', () => { it('sends inbox notification to owner (skip self-notification)', () => { run(claudeDir, ['task', 'comment', '1', '--text', 'Review this', '--from', 'alice']); expect(readInbox(claudeDir, 'bob').length).toBe(1); + const msg = readInbox(claudeDir, 'bob')[0] as Record; + expect(String(msg.text)).toBe('Comment on task #1 "Commentable task":\n\nReview this'); + expect(msg.summary).toBe('Comment on #1'); + expect(msg.source).toBe('system_notification'); run(claudeDir, ['task', 'comment', '1', '--text', 'Self note', '--from', 'bob']); expect(readInbox(claudeDir, 'bob').length).toBe(1); // still 1 @@ -1385,8 +1479,9 @@ describe('teamctl.js', () => { const inbox = readInbox(claudeDir, 'bob'); expect(inbox.length).toBe(1); const text = String((inbox[0] as Record).text); - expect(text).toContain('approved'); - expect(text).toContain('Looks great!'); + expect(text).toBe('Task #1 approved.\n\nLooks great!'); + expect((inbox[0] as Record).summary).toBe('Approved #1'); + expect((inbox[0] as Record).source).toBe('system_notification'); expect((inbox[0] as Record).from).toBe('alice'); }); @@ -1412,8 +1507,15 @@ describe('teamctl.js', () => { expect((readKanban(claudeDir).tasks as Record)['1']).toBeUndefined(); expect(readTask(claudeDir, '1').status).toBe('in_progress'); const text = String((readInbox(claudeDir, 'bob')[0] as Record).text); - expect(text).toContain('Fix the edge case'); - expect(text).toContain('Please fix'); + expect(text).toBe( + 'Task #1 needs fixes.\n\nFix the edge case\n\nPlease fix and mark it as completed when ready.' + ); + expect((readInbox(claudeDir, 'bob')[0] as Record).summary).toBe( + 'Fix request for #1' + ); + expect((readInbox(claudeDir, 'bob')[0] as Record).source).toBe( + 'system_notification' + ); }); it('request-changes without --comment uses default text', () => { diff --git a/test/main/utils/teamNotificationBuilder.test.ts b/test/main/utils/teamNotificationBuilder.test.ts new file mode 100644 index 00000000..7ab28166 --- /dev/null +++ b/test/main/utils/teamNotificationBuilder.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildDetectedErrorFromTeam, + type TeamEventType, + type TeamNotificationPayload, +} from '@main/utils/teamNotificationBuilder'; + +function makePayload(overrides: Partial = {}): TeamNotificationPayload { + return { + teamEventType: 'user_inbox', + teamName: 'my-team', + teamDisplayName: 'My Team', + from: 'alice', + summary: 'Hello from Alice', + body: 'Full message body here', + dedupeKey: 'inbox:my-team:alice:123', + ...overrides, + }; +} + +describe('buildDetectedErrorFromTeam', () => { + it('creates a DetectedError with category "team"', () => { + const result = buildDetectedErrorFromTeam(makePayload()); + expect(result.category).toBe('team'); + }); + + it('sets sessionId as "team:{teamName}"', () => { + const result = buildDetectedErrorFromTeam(makePayload({ teamName: 'alpha-team' })); + expect(result.sessionId).toBe('team:alpha-team'); + }); + + it('sets projectId to teamName', () => { + const result = buildDetectedErrorFromTeam(makePayload({ teamName: 'beta' })); + expect(result.projectId).toBe('beta'); + }); + + it('sets source to teamEventType', () => { + const result = buildDetectedErrorFromTeam(makePayload({ teamEventType: 'task_clarification' })); + expect(result.source).toBe('task_clarification'); + }); + + it('includes dedupeKey', () => { + const result = buildDetectedErrorFromTeam( + makePayload({ dedupeKey: 'clarification:team1:42' }) + ); + expect(result.dedupeKey).toBe('clarification:team1:42'); + }); + + it('sets teamEventType on the result', () => { + const result = buildDetectedErrorFromTeam(makePayload({ teamEventType: 'rate_limit' })); + expect(result.teamEventType).toBe('rate_limit'); + }); + + it('constructs message from "from" and body', () => { + const result = buildDetectedErrorFromTeam( + makePayload({ from: 'bob', body: 'Something happened' }) + ); + expect(result.message).toBe('[bob] Something happened'); + }); + + it('truncates body to 300 chars in message', () => { + const longBody = 'x'.repeat(500); + const result = buildDetectedErrorFromTeam(makePayload({ body: longBody })); + // "[alice] " = 8 chars + 300 chars body = 308 total + expect(result.message.length).toBe(8 + 300); + }); + + it('sets context.projectName to teamDisplayName', () => { + const result = buildDetectedErrorFromTeam(makePayload({ teamDisplayName: 'Alpha Squad' })); + expect(result.context.projectName).toBe('Alpha Squad'); + }); + + it('sets context.cwd to projectPath when provided', () => { + const result = buildDetectedErrorFromTeam(makePayload({ projectPath: '/home/user/project' })); + expect(result.context.cwd).toBe('/home/user/project'); + }); + + it('generates a UUID id', () => { + const result = buildDetectedErrorFromTeam(makePayload()); + expect(result.id).toMatch(/^[0-9a-f-]{36}$/); + }); + + it('sets filePath to empty string', () => { + const result = buildDetectedErrorFromTeam(makePayload()); + expect(result.filePath).toBe(''); + }); + + const EXPECTED_CONFIG: Record = { + rate_limit: { triggerName: 'Rate Limit', triggerColor: 'red' }, + lead_inbox: { triggerName: 'Team Inbox', triggerColor: 'blue' }, + user_inbox: { triggerName: 'User Inbox', triggerColor: 'green' }, + task_clarification: { triggerName: 'Clarification', triggerColor: 'orange' }, + task_status_change: { triggerName: 'Status Change', triggerColor: 'purple' }, + }; + + for (const [eventType, expected] of Object.entries(EXPECTED_CONFIG)) { + it(`maps ${eventType} → triggerName="${expected.triggerName}", triggerColor="${expected.triggerColor}"`, () => { + const result = buildDetectedErrorFromTeam( + makePayload({ teamEventType: eventType as TeamEventType }) + ); + expect(result.triggerName).toBe(expected.triggerName); + expect(result.triggerColor).toBe(expected.triggerColor); + }); + } +}); diff --git a/test/renderer/store/notificationSlice.test.ts b/test/renderer/store/notificationSlice.test.ts index 012e8d17..8d2557e7 100644 --- a/test/renderer/store/notificationSlice.test.ts +++ b/test/renderer/store/notificationSlice.test.ts @@ -458,6 +458,52 @@ describe('notificationSlice', () => { }); }); + describe('team notification navigation', () => { + it('should open team tab for any team notification (sessionId starts with "team:")', () => { + const teamError = createMockError({ + sessionId: 'team:alpha-team', + source: 'user_inbox', + category: 'team' as never, + teamEventType: 'user_inbox' as never, + }); + + store.getState().navigateToError(teamError); + + // Should open a team tab, not a session tab + const tabs = store.getState().openTabs; + expect(tabs).toHaveLength(1); + expect(tabs[0].type).toBe('team'); + }); + + it('should open team tab for rate-limit notification with team sessionId', () => { + const teamError = createMockError({ + sessionId: 'team:beta-team', + source: 'rate_limit', + category: 'team' as never, + teamEventType: 'rate_limit' as never, + }); + + store.getState().navigateToError(teamError); + + const tabs = store.getState().openTabs; + expect(tabs).toHaveLength(1); + expect(tabs[0].type).toBe('team'); + }); + + it('should open team tab regardless of source field value', () => { + const teamError = createMockError({ + sessionId: 'team:gamma-team', + source: 'task_clarification', + }); + + store.getState().navigateToError(teamError); + + const tabs = store.getState().openTabs; + expect(tabs).toHaveLength(1); + expect(tabs[0].type).toBe('team'); + }); + }); + describe('existing tab behavior', () => { it('should focus existing tab if session is already open', () => { // Open target session tab first