feat: enhance review approval process and notification handling

- Added a `suppressTaskComment` flag to the `approveReview` function, allowing users to approve reviews without adding a comment to the task.
- Updated the `notifyNewInboxMessages` function to include additional debug logging for better tracking of inbox notifications.
- Adjusted notification settings in `ConfigManager` to enable notifications for lead inbox and change the status change behavior.
- Enhanced the `NotificationManager` to improve error handling and logging during notification display.
- Refactored `TeamDataService` to utilize the new `suppressTaskComment` feature during review approvals.
- Updated tests to validate the new approval process and notification behaviors.

agent-teams-controller/src/internal/review.js

@@ -109,6 +109,7 @@ function approveReview(context, taskId, flags = {}) {
   const from =
     typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'team-lead';
   const note = typeof flags.note === 'string' && flags.note.trim() ? flags.note.trim() : 'Approved';
+  const suppressTaskComment = flags.suppressTaskComment === true;
   const leadSessionId = resolveLeadSessionId(context, flags);
   const prevReviewState = getCurrentReviewState(task);
 
@@ -127,12 +128,14 @@ function approveReview(context, taskId, flags = {}) {
     return t;
   });
 
-  tasks.addTaskComment(context, task.id, {
-    text: note,
-    from,
-    type: 'review_approved',
-    notifyOwner: false,
-  });
+  if (!suppressTaskComment) {
+    tasks.addTaskComment(context, task.id, {
+      text: note,
+      from,
+      type: 'review_approved',
+      notifyOwner: false,
+    });
+  }
 
   if ((flags.notify === true || flags['notify-owner'] === true) && task.owner) {
     messages.sendMessage(context, {

src/main/index.ts

@@ -183,6 +183,7 @@ function extractNotificationContent(text: string): { summary: string; body: stri
 }
 
 async function notifyNewInboxMessages(teamName: string, detail: string): Promise<void> {
+  logger.debug(`[inbox-notify] called: team=${teamName} detail=${detail}`);
   const config = configManager.getConfig();
 
   // Skip orphaned team directories without config.json (e.g., "default").
@@ -191,6 +192,7 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise
   // correct team name via sentMessages.json, so inbox notifications from orphaned dirs
   // would be duplicates with a wrong team name.
   if (!existsSync(join(getTeamsBasePath(), teamName, 'config.json'))) {
+    logger.debug(`[inbox-notify] skipped: no config.json for team=${teamName}`);
     return; // No config.json → orphaned team dir, skip notification
   }
 
@@ -221,6 +223,7 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise
 
     if (isFirstLoad) {
       // First load — seed count, don't notify for pre-existing messages
+      logger.debug(`[inbox-notify] first load for ${key}: seeding count=${messages.length}`);
       inboxMessageCounts.set(key, messages.length);
       return;
     }
@@ -234,6 +237,10 @@ async function notifyNewInboxMessages(teamName: string, detail: string): Promise
     const newMessages = messages.slice(0, messages.length - prevCount);
     inboxMessageCounts.set(key, messages.length);
 
+    logger.debug(
+      `[inbox-notify] ${key}: prevCount=${prevCount} newCount=${messages.length} newMessages=${newMessages.length} suppressToast=${String(suppressToast)}`
+    );
+
     const teamDisplayName = await resolveTeamDisplayName(teamName);
 
     for (let i = 0; i < newMessages.length; i++) {
@@ -729,9 +736,11 @@ function initializeServices(): void {
   // Fire-and-forget: initializeServices() is sync, cannot await.
   // Safe because TeamBackupService.initialized flag blocks all backup/restore
   // operations until initialize() completes internally (restore → prune → set flag).
-  void teamBackupService.initialize().catch((error: unknown) =>
-    logger.warn(`[Init] TeamBackupService init failed: ${String(error)}`)
-  );
+  void teamBackupService
+    .initialize()
+    .catch((error: unknown) =>
+      logger.warn(`[Init] TeamBackupService init failed: ${String(error)}`)
+    );
 
   // Cross-team communication service
   const crossTeamConfigReader = new TeamConfigReader();

src/main/services/infrastructure/ConfigManager.ts

@@ -259,12 +259,12 @@ const DEFAULT_CONFIG: AppConfig = {
     snoozedUntil: null,
     snoozeMinutes: 30,
     includeSubagentErrors: false,
-    notifyOnLeadInbox: false,
+    notifyOnLeadInbox: true,
     notifyOnUserInbox: true,
     notifyOnClarifications: true,
     notifyOnStatusChange: true,
     notifyOnTaskComments: true,
-    statusChangeOnlySolo: true,
+    statusChangeOnlySolo: false,
     statusChangeStatuses: ['in_progress', 'completed'],
     triggers: DEFAULT_TRIGGERS,
   },

src/main/services/infrastructure/NotificationManager.ts

@@ -421,41 +421,56 @@ export class NotificationManager extends EventEmitter {
     stored: StoredNotification,
     payload: TeamNotificationPayload
   ): void {
-    if (!this.isNativeNotificationSupported()) return;
+    if (!this.isNativeNotificationSupported()) {
+      logger.warn('[team-toast] native notifications not supported — skipping');
+      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 } : {}),
-    });
+    try {
+      const config = this.configManager.getConfig();
+      const isMac = process.platform === 'darwin';
+      const truncatedBody = stripMarkdown(payload.body).slice(0, 300);
+      const iconPath = isMac ? undefined : getAppIconPath();
 
-    // Hold a strong reference to prevent GC from collecting the notification
-    this.activeNotifications.add(notification);
-    const cleanup = (): void => {
-      this.activeNotifications.delete(notification);
-    };
+      logger.debug(
+        `[team-toast] creating: title="${payload.teamDisplayName}" summary="${payload.summary ?? ''}" bodyLen=${truncatedBody.length}`
+      );
 
-    notification.on('click', () => {
-      this.handleNativeNotificationClick(stored);
-      cleanup();
-    });
-    notification.on('close', cleanup);
+      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('show', () => {
-      logger.debug(`[notification] shown: "${payload.teamDisplayName}" — ${payload.summary ?? ''}`);
-    });
-    notification.on('failed', (_, error) => {
-      logger.warn(`[notification] failed: ${error}`);
-      cleanup();
-    });
+      // Hold a strong reference to prevent GC from collecting the notification
+      this.activeNotifications.add(notification);
+      const cleanup = (): void => {
+        this.activeNotifications.delete(notification);
+      };
 
-    notification.show();
+      notification.on('click', () => {
+        this.handleNativeNotificationClick(stored);
+        cleanup();
+      });
+      notification.on('close', cleanup);
+
+      notification.on('show', () => {
+        logger.debug(
+          `[team-toast] OS confirmed show: "${payload.teamDisplayName}" — ${payload.summary ?? ''}`
+        );
+      });
+      notification.on('failed', (_, error) => {
+        logger.warn(`[team-toast] OS failed: ${String(error)}`);
+        cleanup();
+      });
+
+      notification.show();
+      logger.debug('[team-toast] notification.show() called');
+    } catch (error) {
+      logger.error(`[team-toast] exception in showTeamNativeNotification: ${String(error)}`);
+    }
   }
 
   /**
@@ -653,10 +668,21 @@ export class NotificationManager extends EventEmitter {
   async addTeamNotification(payload: TeamNotificationPayload): Promise<StoredNotification | null> {
     const error = buildDetectedErrorFromTeam(payload);
     const stored = await this.storeNotification(error);
-    if (!stored) return null;
+    if (!stored) {
+      logger.debug(
+        `[team-notification] skipped (dedup): type=${payload.teamEventType} key=${payload.dedupeKey}`
+      );
+      return null;
+    }
 
     // Team-specific toast policy: enabled/snoozed + suppressToast + dedupeKey throttle only
-    if (!payload.suppressToast && this.areNotificationsEnabled() && !this.isToastThrottled(error)) {
+    const enabled = this.areNotificationsEnabled();
+    const throttled = this.isToastThrottled(error);
+    const shouldShow = !payload.suppressToast && enabled && !throttled;
+    logger.debug(
+      `[team-notification] toast decision: type=${payload.teamEventType} suppressToast=${String(payload.suppressToast ?? false)} enabled=${String(enabled)} throttled=${String(throttled)} → show=${String(shouldShow)}`
+    );
+    if (shouldShow) {
       this.showTeamNativeNotification(stored, payload);
     }
 

src/main/services/team/TeamDataService.ts

@@ -1891,7 +1891,7 @@ export class TeamDataService {
         const { leadSessionId } = await this.resolveLeadRuntimeContext(teamName);
         controller.review.approveReview(taskId, {
           from: 'user',
-          note: 'Approved from kanban',
+          suppressTaskComment: true,
           'notify-owner': true,
           ...(leadSessionId ? { leadSessionId } : {}),
         });

src/renderer/components/team/dialogs/TaskDetailDialog.tsx

@@ -224,8 +224,11 @@ export const TaskDetailDialog = ({
     lightboxOpenRef.current = isOpen;
   }, []);
 
-  // Ref for the scrollable DialogContent — needed as IO root for viewport-based read tracking.
-  const dialogContentRef = useRef<HTMLDivElement | null>(null);
+  // Callback-ref + useState for the scrollable DialogContent — needed as IO root
+  // for viewport-based read tracking. Using useState (not useRef) ensures that
+  // useViewportObserver recreates the IntersectionObserver when the portal mounts
+  // and the DOM element becomes available.
+  const [dialogContentEl, setDialogContentEl] = useState<HTMLDivElement | null>(null);
   const handleReply = useCallback(
     (author: string, text: string) => {
       if (currentTask) setReplyTo({ taskId: currentTask.id, author, text });
@@ -269,7 +272,7 @@ export const TaskDetailDialog = ({
   const { registerComment, flush: flushCommentRead } = useViewportCommentRead({
     teamName,
     taskId: currentTask?.id ?? '',
-    scrollContainerRef: dialogContentRef,
+    scrollContainer: dialogContentEl,
   });
 
   const handleClose = useCallback(() => {
@@ -525,7 +528,7 @@ export const TaskDetailDialog = ({
       }}
     >
       <DialogContent
-        ref={dialogContentRef}
+        ref={setDialogContentEl}
         className="sm:min-w-[500px] sm:max-w-4xl"
         onInteractOutside={(e) => {
           if (lightboxOpenRef.current) e.preventDefault();

src/renderer/components/team/members/MemberLogsTab.tsx

@@ -65,10 +65,6 @@ function filterChunksByWorkIntervals(
       return cs <= end && ce >= i.startMs;
     });
   });
-  // DEBUG
-  console.log(
-    `[filterChunks] intervals=${parsed.length} chunks=${chunks.length}→${filtered.length}`
-  );
   return filtered;
 }
 
@@ -111,12 +107,6 @@ export const MemberLogsTab = ({
   showLeadPreview = false,
   onPreviewOnlineChange,
 }: MemberLogsTabProps): React.JSX.Element => {
-  // DEBUG: verify workIntervals reach this component
-  if (taskId && taskWorkIntervals) {
-    console.log(
-      `[MemberLogsTab] taskId=${taskId} workIntervals=${JSON.stringify(taskWorkIntervals)}`
-    );
-  }
   const MIN_REFRESH_VISIBLE_MS = 250;
   const intervalsKey = useMemo(
     () => (taskWorkIntervals ? JSON.stringify(taskWorkIntervals) : ''),
@@ -198,21 +188,52 @@ export const MemberLogsTab = ({
       if (!Number.isFinite(startMs)) return Number.NaN;
       const durationMs = Number.isFinite(log.durationMs) ? Math.max(0, log.durationMs) : 0;
       const endMs = startMs + durationMs;
-      // Keep actively-updating logs at the top even if duration lags slightly.
       return log.isOngoing ? Math.max(endMs, nowMs) : endMs;
     };
 
-    const withIndex = logs.map((log, index) => ({ log, index }));
+    // When viewing a task with workIntervals, sort by overlap (most relevant first).
+    // Fallback to endMs (most recent activity) when no intervals available.
+    const getOverlapMs = (log: MemberLogSummary): number => {
+      if (!taskWorkIntervals || taskWorkIntervals.length === 0) return 0;
+      const logStartMs = new Date(log.startTime).getTime();
+      if (!Number.isFinite(logStartMs)) return 0;
+      const logDurationMs = Number.isFinite(log.durationMs) ? Math.max(0, log.durationMs) : 0;
+      const logEndMs = log.isOngoing ? nowMs : logStartMs + logDurationMs;
+
+      let totalOverlap = 0;
+      for (const interval of taskWorkIntervals) {
+        const intStart = Date.parse(interval.startedAt);
+        if (!Number.isFinite(intStart)) continue;
+        const intEnd =
+          typeof interval.completedAt === 'string' ? Date.parse(interval.completedAt) : nowMs;
+        if (!Number.isFinite(intEnd)) continue;
+        const overlapStart = Math.max(logStartMs, intStart);
+        const overlapEnd = Math.min(logEndMs, intEnd);
+        if (overlapEnd > overlapStart) totalOverlap += overlapEnd - overlapStart;
+      }
+      return totalOverlap;
+    };
+
+    const withIndex = logs.map((log, index) => ({
+      log,
+      index,
+      overlap: getOverlapMs(log),
+      lastActivity: getLastActivityMs(log),
+    }));
+
     withIndex.sort((a, b) => {
-      const aTime = getLastActivityMs(a.log);
-      const bTime = getLastActivityMs(b.log);
+      // Primary: overlap with task workIntervals (more overlap = higher)
+      if (a.overlap !== b.overlap) return b.overlap - a.overlap;
+      // Secondary: last activity (most recent first)
+      const aTime = a.lastActivity;
+      const bTime = b.lastActivity;
       if (Number.isFinite(aTime) && Number.isFinite(bTime) && aTime !== bTime) return bTime - aTime;
       if (Number.isFinite(aTime) && !Number.isFinite(bTime)) return -1;
       if (!Number.isFinite(aTime) && Number.isFinite(bTime)) return 1;
       return a.index - b.index;
     });
     return withIndex.map((x) => x.log);
-  }, [logs]);
+  }, [logs, taskWorkIntervals]);
 
   const shouldShowPreview = useMemo(() => {
     return taskId != null && (showSubagentPreview || showLeadPreview);

src/renderer/components/ui/auto-resize-textarea.tsx

@@ -40,12 +40,26 @@ const AutoResizeTextarea = React.forwardRef<HTMLTextAreaElement, AutoResizeTexta
       const maxHeight =
         maxRows * lineHeight + paddingTop + paddingBottom + borderTop + borderBottom;
 
+      // Save scrollTop before collapsing height — setting height='auto'
+      // causes the browser to reset scrollTop to 0.
+      const savedScrollTop = textarea.scrollTop;
+
       textarea.style.height = 'auto';
       const scrollHeight = textarea.scrollHeight;
       const clampedHeight = Math.min(Math.max(scrollHeight, minHeight), maxHeight);
 
       textarea.style.height = `${clampedHeight}px`;
       textarea.style.overflowY = scrollHeight > maxHeight ? 'auto' : 'hidden';
+
+      // Restore scrollTop after height adjustment, then scroll to caret
+      // if cursor is at the end (common after paste).
+      if (scrollHeight > maxHeight) {
+        if (textarea.selectionStart === textarea.value.length) {
+          textarea.scrollTop = textarea.scrollHeight;
+        } else {
+          textarea.scrollTop = savedScrollTop;
+        }
+      }
     }, [minRows, maxRows]);
 
     React.useEffect(() => {

src/renderer/hooks/useViewportCommentRead.ts

@@ -4,17 +4,17 @@ import { markCommentsRead } from '@renderer/services/commentReadStorage';
 
 import { useViewportObserver } from './useViewportObserver';
 
-import type { RefObject } from 'react';
-
 interface UseViewportCommentReadOptions {
   teamName: string;
   taskId: string;
   /**
-   * Scrollable ancestor element (e.g. DialogContent) used as IO root.
-   * Required for portalled Dialogs where the default viewport root
-   * would not detect intersections correctly.
+   * Scrollable ancestor DOM element (e.g. DialogContent) used as IO root.
+   * Pass the actual element (not a RefObject) so that the observer is
+   * recreated when the portal mounts. Use useState + callback ref:
+   *   const [rootEl, setRootEl] = useState<HTMLDivElement | null>(null);
+   *   <DialogContent ref={setRootEl}>
    */
-  scrollContainerRef: RefObject<HTMLElement | null>;
+  scrollContainer: HTMLElement | null;
 }
 
 /**
@@ -34,7 +34,7 @@ interface UseViewportCommentReadOptions {
 export function useViewportCommentRead({
   teamName,
   taskId,
-  scrollContainerRef,
+  scrollContainer,
 }: UseViewportCommentReadOptions): {
   /** Ref callback factory. Call with the comment's unique ID. */
   registerComment: (commentId: string) => (el: HTMLElement | null) => void;
@@ -78,7 +78,7 @@ export function useViewportCommentRead({
   );
 
   const { registerElement } = useViewportObserver({
-    rootRef: scrollContainerRef,
+    root: scrollContainer,
     threshold: 0.1,
     onVisibleChange: handleVisibleChange,
   });

src/renderer/hooks/useViewportObserver.ts

@@ -1,17 +1,20 @@
 import { useCallback, useEffect, useRef } from 'react';
 
-import type { RefObject } from 'react';
-
 /** Data attribute name used to store arbitrary string data on observed elements. */
 const DATA_ATTR = 'data-viewport-value';
 
 interface UseViewportObserverOptions {
   /**
-   * Scrollable ancestor element used as IntersectionObserver root.
-   * Required for elements inside Dialog portals where the default
-   * document viewport root would not detect intersections correctly.
+   * Scrollable ancestor DOM element used as IntersectionObserver root.
+   * Pass the actual element (not a RefObject) so that the hook can
+   * react to the element becoming available (e.g. after a Dialog portal mounts).
+   *
+   * Use a callback-ref + useState pattern in the consumer:
+   *   const [rootEl, setRootEl] = useState<HTMLElement | null>(null);
+   *   <DialogContent ref={setRootEl}>
+   *   useViewportObserver({ root: rootEl, ... })
    */
-  rootRef?: RefObject<HTMLElement | null>;
+  root?: HTMLElement | null;
   /** Visibility ratio threshold (0..1). Default: 0.1 (10% visible). */
   threshold?: number;
   /**
@@ -26,17 +29,21 @@ interface UseViewportObserverOptions {
  * scrollable container using IntersectionObserver.
  *
  * Usage:
- * 1. Call the hook with a root ref and a callback.
+ * 1. Call the hook with a root element and a callback.
  * 2. Attach `registerElement(value)` as a ref callback on each element.
  *    `value` is an arbitrary string stored in a data attribute for identification.
  * 3. The callback fires with the list of currently visible values whenever
  *    the intersection state changes.
  *
+ * Important: pass the root as a plain DOM element (not a RefObject) so the
+ * hook can recreate the observer when the element becomes available.
+ * Use `useState` + callback ref in the consumer for this.
+ *
  * The hook manages a single IntersectionObserver instance and handles
  * element registration/deregistration automatically.
  */
 export function useViewportObserver({
-  rootRef,
+  root,
   threshold = 0.1,
   onVisibleChange,
 }: UseViewportObserverOptions): {
@@ -50,9 +57,15 @@ export function useViewportObserver({
   const visibleValuesRef = useRef<Set<string>>(new Set());
   const elementsByValue = useRef<Map<string, HTMLElement>>(new Map());
 
-  // Create / recreate observer when root or threshold changes.
+  // Create / recreate observer when root element or threshold changes.
+  // root is a plain DOM element (not a RefObject), so when the consumer
+  // updates state (e.g. Dialog portal mounts), this effect re-runs and
+  // creates an IO with the correct root.
   useEffect(() => {
-    const root = rootRef?.current ?? null;
+    // When root is not yet available (e.g. portal not mounted), skip
+    // creating the observer — it would default to document viewport
+    // and produce false positives for all visible elements.
+    if (!root) return;
 
     const observer = new IntersectionObserver(
       (entries) => {
@@ -94,7 +107,7 @@ export function useViewportObserver({
       observerRef.current = null;
       visibleValuesRef.current.clear();
     };
-  }, [rootRef, threshold]);
+  }, [root, threshold]);
 
   const registerElement = useCallback((value: string) => {
     return (el: HTMLElement | null) => {

test/main/services/team/TeamDataService.test.ts

@@ -566,7 +566,7 @@ describe('TeamDataService', () => {
     });
     expect(approveReviewMock).toHaveBeenCalledWith('task-1', {
       from: 'user',
-      note: 'Approved from kanban',
+      suppressTaskComment: true,
       'notify-owner': true,
       leadSessionId: 'lead-2',
     });