feat: enhance file path validation and member stats computation

- Refactored isValidFilePath function to improve validation logic, including trimming whitespace and stripping trailing punctuation. - Updated MemberStatsComputer to filter out invalid file paths from perFileStats, ensuring only valid paths are included in the final stats. - Added unit tests for isValidFilePath to cover various edge cases, enhancing reliability of file path handling. - Improved overall code readability and maintainability in MemberStatsComputer.
2026-03-06 13:00:08 +02:00 · 2026-03-06 13:00:08 +02:00 · 30b4e74924
commit 30b4e74924
parent b093e87c89
11 changed files with 185 additions and 124 deletions
--- a/src/main/services/team/MemberStatsComputer.ts
+++ b/src/main/services/team/MemberStatsComputer.ts
@ -9,8 +9,12 @@ import type { FileLineStats, MemberFullStats } from '@shared/types';

 const logger = createLogger('Service:MemberStatsComputer');

-function isValidFilePath(value: string): boolean {
-  return value.length > 0 && value !== 'null' && value !== 'undefined' && value !== 'None';
+const TRAILING_PUNCT = /[;.,]+$/;
+const INVALID_NAMES = new Set(['null', 'undefined', 'None', 'false', 'true', '']);
+
+export function isValidFilePath(value: string): boolean {
+  const cleaned = value.trim().replace(TRAILING_PUNCT, '');
+  return cleaned.length > 1 && !INVALID_NAMES.has(cleaned) && cleaned.includes('/');
 }

 const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
@ -73,11 +77,19 @@ export class MemberStatsComputer {
      .filter(isValidFilePath)
      .sort((a, b) => a.localeCompare(b));

+    // Also filter perFileStats keys to exclude invalid paths
+    const filteredFileStats: Record<string, FileLineStats> = {};
+    for (const [fp, fls] of Object.entries(perFileStats)) {
+      if (isValidFilePath(fp)) {
+        filteredFileStats[fp] = fls;
+      }
+    }
+
    const stats: MemberFullStats = {
      linesAdded,
      linesRemoved,
      filesTouched: validFiles,
-      fileStats: perFileStats,
+      fileStats: filteredFileStats,
      toolUsage,
      inputTokens,
      outputTokens,
@ -121,18 +133,24 @@ export class MemberStatsComputer {
    // Track last known content per file for accurate Write/NotebookEdit diffs
    const fileLastContent = new Map<string, string>();

+    const cleanPath = (fp: string): string => fp.trim().replace(TRAILING_PUNCT, '');
+
    const trackFile = (fp: string): void => {
-      if (typeof fp === 'string' && isValidFilePath(fp)) filesTouchedSet.add(fp);
+      if (typeof fp === 'string') {
+        const cleaned = cleanPath(fp);
+        if (isValidFilePath(cleaned)) filesTouchedSet.add(cleaned);
+      }
    };

    const addFileLines = (fp: string, added: number, removed: number): void => {
-      if (!isValidFilePath(fp)) return;
-      const existing = perFileStats[fp];
+      const cleaned = cleanPath(fp);
+      if (!isValidFilePath(cleaned)) return;
+      const existing = perFileStats[cleaned];
      if (existing) {
        existing.added += added;
        existing.removed += removed;
      } else {
-        perFileStats[fp] = { added, removed };
+        perFileStats[cleaned] = { added, removed };
      }
    };

--- a/src/main/services/team/TeamDataService.ts
+++ b/src/main/services/team/TeamDataService.ts
@ -9,7 +9,11 @@ import {
 } from '@main/utils/pathDecoder';
 import { isProcessAlive } from '@main/utils/processHealth';
 import { killProcessByPid } from '@main/utils/processKill';
-import { AGENT_BLOCK_CLOSE, AGENT_BLOCK_OPEN } from '@shared/constants/agentBlocks';
+import {
+  AGENT_BLOCK_CLOSE,
+  AGENT_BLOCK_OPEN,
+  stripAgentBlocks,
+} from '@shared/constants/agentBlocks';
 import { getMemberColor } from '@shared/constants/memberColors';
 import { createLogger } from '@shared/utils/logger';
 import { parseNumericSuffixName } from '@shared/utils/teamMemberName';
@ -60,7 +64,7 @@ import type {
 const logger = createLogger('Service:TeamDataService');

 const MIN_TEXT_LENGTH = 30;
-const MAX_LEAD_TEXTS = 50;
+const MAX_LEAD_TEXTS = 150;
 const PROCESS_HEALTH_INTERVAL_MS = 2_000;
 const MAX_PROCESSES_FILE_BYTES = 2 * 1024 * 1024;
 const TASK_MAP_YIELD_EVERY = 250;
@ -1472,20 +1476,31 @@ export class TeamDataService {
          const timestamp =
            typeof msg.timestamp === 'string' ? msg.timestamp : new Date().toISOString();

+          const textParts: string[] = [];
          for (const block of content as Record<string, unknown>[]) {
            if (block.type !== 'text' || typeof block.text !== 'string') continue;
-            const text = block.text.trim();
-            if (text.length < MIN_TEXT_LENGTH) continue;
-            textsReversed.push({
-              from: leadName,
-              text,
-              timestamp,
-              read: true,
-              source: 'lead_session',
-              leadSessionId: config.leadSessionId,
-            });
-            if (textsReversed.length >= MAX_LEAD_TEXTS) break;
+            textParts.push(block.text);
          }
+          if (textParts.length === 0) continue;
+
+          const combined = stripAgentBlocks(textParts.join('\n')).trim();
+          if (combined.length < MIN_TEXT_LENGTH) continue;
+
+          // Stable messageId: timestamp + text prefix (survives tail-scan range changes)
+          const textPrefix = combined
+            .slice(0, 50)
+            .replace(/[^\p{L}\p{N}]/gu, '')
+            .slice(0, 20);
+
+          textsReversed.push({
+            from: leadName,
+            text: combined,
+            timestamp,
+            read: true,
+            source: 'lead_session',
+            leadSessionId: config.leadSessionId,
+            messageId: `lead-session-${timestamp}-${textPrefix}`,
+          });
          if (textsReversed.length >= MAX_LEAD_TEXTS) break;
        }

--- a/src/main/services/team/TeamProvisioningService.ts
+++ b/src/main/services/team/TeamProvisioningService.ts
@ -161,15 +161,8 @@ interface ProvisioningRun {
    rejectOnce: (error: string) => void;
    timeoutHandle: NodeJS.Timeout;
  } | null;
-  /**
-   * Accumulates assistant text for direct user→lead messages (no relay capture active).
-   * Flushed to liveLeadProcessMessages on result.success.
-   */
-  directReplyParts: string[];
-  /** Monotonic counter for stream-json turns (incremented on result). */
-  leadTurnSeq: number;
-  /** Stable timestamp used for the current aggregated lead turn message. */
-  leadTurnMessageTimestamp: string | null;
+  /** Monotonic counter for individual lead assistant messages. */
+  leadMsgSeq: number;
  /** Throttle timestamp for emitting inbox refresh events for lead text. */
  lastLeadTextEmitMs: number;
  /**
@ -1750,9 +1743,7 @@ export class TeamProvisioningService {
        isLaunch: false,
        fsPhase: 'waiting_config',
        leadRelayCapture: null,
-        directReplyParts: [],
-        leadTurnSeq: 0,
-        leadTurnMessageTimestamp: null,
+        leadMsgSeq: 0,
        lastLeadTextEmitMs: 0,
        silentUserDmForward: null,
        silentUserDmForwardClearHandle: null,
@ -2051,9 +2042,7 @@ export class TeamProvisioningService {
        isLaunch: true,
        fsPhase: 'waiting_members',
        leadRelayCapture: null,
-        directReplyParts: [],
-        leadTurnSeq: 0,
-        leadTurnMessageTimestamp: null,
+        leadMsgSeq: 0,
        lastLeadTextEmitMs: 0,
        silentUserDmForward: null,
        silentUserDmForwardClearHandle: null,
@ -2491,8 +2480,6 @@ export class TeamProvisioningService {
          },
        };
        run.leadRelayCapture = capture;
-        // Clear any direct reply parts — relay capture takes priority
-        run.directReplyParts = [];
      });

      try {
@ -2921,27 +2908,21 @@ export class TeamProvisioningService {
            }, capture.idleMs);
          }
        } else if (run.provisioningComplete) {
-          // Accumulate assistant text for a single "live lead turn" message in Messages.
-          // If the same assistant message includes SendMessage(to:"user"), prefer the captured
-          // SendMessage and avoid duplicating it as a separate lead text entry.
+          // Push each assistant text block as a separate live message (per-message pattern).
+          // When the same assistant message includes SendMessage(to:"user"), skip text —
+          // captureSendMessageToUser() handles it separately.
          if (!run.silentUserDmForward && !hasSendMessageToUser) {
-            run.directReplyParts.push(text);
-            const raw = run.directReplyParts.join('\n');
-            const cleanText = stripAgentBlocks(raw).trim();
+            const cleanText = stripAgentBlocks(text).trim();
            if (cleanText.length >= TeamProvisioningService.LEAD_TEXT_MIN_LENGTH) {
+              run.leadMsgSeq += 1;
              const leadName =
                run.request.members.find((m) => m.role?.toLowerCase().includes('lead'))?.name ||
                'team-lead';
-              if (!run.leadTurnMessageTimestamp) {
-                run.leadTurnMessageTimestamp = nowIso();
-              }
-              // Update timestamp on each text block so the live indicator stays fresh
-              const currentTimestamp = nowIso();
-              const messageId = `lead-turn-${run.runId}-${run.leadTurnSeq}`;
+              const messageId = `lead-turn-${run.runId}-${run.leadMsgSeq}`;
              const leadMsg: InboxMessage = {
                from: leadName,
                text: cleanText,
-                timestamp: currentTimestamp,
+                timestamp: nowIso(),
                read: true,
                summary: cleanText.length > 60 ? cleanText.slice(0, 57) + '...' : cleanText,
                messageId,
@ -2962,13 +2943,6 @@ export class TeamProvisioningService {
                });
              }
            }
-          } else if (hasSendMessageToUser) {
-            run.directReplyParts = [];
-            run.leadTurnMessageTimestamp = null;
-            this.removeLiveLeadProcessMessage(
-              run.teamName,
-              `lead-turn-${run.runId}-${run.leadTurnSeq}`
-            );
          }
        }
      }
@ -3110,47 +3084,6 @@ export class TeamProvisioningService {
          const capture = run.leadRelayCapture;
          const combined = capture.textParts.join('\n').trim();
          capture.resolveOnce(combined);
-        } else if (run.provisioningComplete && run.directReplyParts.length > 0) {
-          // Finalize the current live lead turn message (single messageId per turn).
-          const rawReply = run.directReplyParts.join('\n').trim();
-          run.directReplyParts = [];
-          const leadName =
-            run.request.members.find((m) => m.role?.toLowerCase().includes('lead'))?.name ||
-            'team-lead';
-          const replyText = stripAgentBlocks(rawReply).trim();
-          const finalText =
-            replyText.length > 0
-              ? replyText
-              : rawReply.length > 0
-                ? '(Message received and processed)'
-                : '';
-          if (finalText.length > 0) {
-            if (!run.leadTurnMessageTimestamp) {
-              run.leadTurnMessageTimestamp = nowIso();
-            }
-            const messageId = `lead-turn-${run.runId}-${run.leadTurnSeq}`;
-            const replyMsg: InboxMessage = {
-              from: leadName,
-              text: finalText,
-              timestamp: run.leadTurnMessageTimestamp,
-              read: true,
-              summary: finalText.length > 60 ? finalText.slice(0, 57) + '...' : finalText,
-              messageId,
-              source: 'lead_process',
-            };
-            this.pushLiveLeadProcessMessage(run.teamName, replyMsg);
-            this.teamChangeEmitter?.({
-              type: 'inbox',
-              teamName: run.teamName,
-              detail: 'lead-turn-final',
-            });
-          }
-        }
-        // Turn boundary: advance lead turn sequence.
-        if (run.provisioningComplete) {
-          run.leadTurnSeq += 1;
-          run.leadTurnMessageTimestamp = null;
-          run.directReplyParts = [];
        }
        // Clear silent relay flag after any successful turn.
        run.silentUserDmForward = null;
@ -3168,12 +3101,6 @@ export class TeamProvisioningService {
        if (run.leadRelayCapture) {
          run.leadRelayCapture.rejectOnce(errorMsg);
        }
-        // Turn boundary: advance lead turn sequence.
-        if (run.provisioningComplete) {
-          run.leadTurnSeq += 1;
-          run.leadTurnMessageTimestamp = null;
-          run.directReplyParts = [];
-        }
        // Clear silent relay flag after any errored turn.
        run.silentUserDmForward = null;
        if (run.silentUserDmForwardClearHandle) {
--- a/src/renderer/components/layout/SortableTab.tsx
+++ b/src/renderer/components/layout/SortableTab.tsx
@ -125,7 +125,7 @@ export const SortableTab = ({
      role="tab"
      tabIndex={0}
      aria-selected={isActive}
-      className="group flex min-w-0 max-w-[200px] shrink-0 cursor-grab items-center gap-2 rounded-md px-3 py-1.5"
+      className="group flex min-w-0 cursor-grab items-center gap-2 rounded-md px-3 py-1.5"
      style={style}
      onClick={(e) => onTabClick(tab.id, e)}
      onMouseDown={(e) => onMouseDown(tab.id, e)}
@ -188,7 +188,7 @@ export const DragOverlayTab = ({ tab }: { tab: Tab }): React.JSX.Element => {

  return (
    <div
-      className="flex min-w-0 max-w-[200px] items-center gap-2 rounded-md border-2 px-3 py-1.5"
+      className="flex min-w-0 items-center gap-2 rounded-md border-2 px-3 py-1.5"
      style={{
        backgroundColor: 'var(--color-surface-raised)',
        borderColor: 'var(--color-accent, #6366f1)',
--- a/src/renderer/components/team/activity/LeadThoughtsGroup.tsx
+++ b/src/renderer/components/team/activity/LeadThoughtsGroup.tsx
@ -221,34 +221,59 @@ export const LeadThoughtsGroupRow = ({
            {thoughts.length} thoughts
          </span>
          <span className="text-[10px]" style={{ color: CARD_ICON_MUTED }}>
-            {formatTime(oldest.timestamp)}–{formatTime(newest.timestamp)}
+            {formatTime(oldest.timestamp) === formatTime(newest.timestamp)
+              ? formatTime(oldest.timestamp)
+              : `${formatTime(oldest.timestamp)}–${formatTime(newest.timestamp)}`}
          </span>
        </div>

        {/* Scrollable body — fixed height, always visible */}
        <div
          ref={scrollRef}
-          className="space-y-px border-t px-3 py-1.5"
+          className="border-t"
          style={{
            borderColor: 'var(--color-border-subtle)',
            maxHeight: '200px',
-            overflowY: 'auto',
+            overflowY: 'scroll',
            scrollbarWidth: 'thin',
            scrollbarColor: 'var(--scrollbar-thumb) transparent',
          }}
          onScroll={handleScroll}
        >
          {chronologicalThoughts.map((thought, idx) => (
-            <div key={thought.messageId ?? idx} className="flex gap-2 py-0.5 text-[11px]">
-              <span className="shrink-0 font-mono" style={{ color: CARD_ICON_MUTED }}>
-                {formatTimeWithSec(thought.timestamp)}
-              </span>
-              <div className="min-w-0 flex-1 [&_>div>div]:p-0" style={{ color: CARD_TEXT_LIGHT }}>
-                <MarkdownViewer
-                  content={thought.text.replace(/\n/g, '  \n')}
-                  maxHeight="max-h-none"
-                  bare
-                />
+            <div key={thought.messageId ?? idx} className="thought-expand-in">
+              {idx > 0 && (
+                <div className="mx-auto flex w-[40%] items-center justify-center gap-[5px] py-px">
+                  <hr
+                    className="flex-1 border-0"
+                    style={{
+                      height: '1px',
+                      backgroundColor: 'var(--color-border-emphasis)',
+                    }}
+                  />
+                  <span
+                    className="shrink-0 font-mono text-[9px]"
+                    style={{ color: CARD_ICON_MUTED }}
+                  >
+                    {formatTimeWithSec(thought.timestamp)}
+                  </span>
+                  <hr
+                    className="flex-1 border-0"
+                    style={{
+                      height: '1px',
+                      backgroundColor: 'var(--color-border-emphasis)',
+                    }}
+                  />
+                </div>
+              )}
+              <div className="flex text-[11px]">
+                <div className="min-w-0 flex-1 [&_>div>div]:p-0" style={{ color: CARD_TEXT_LIGHT }}>
+                  <MarkdownViewer
+                    content={thought.text.replace(/\n/g, '  \n')}
+                    maxHeight="max-h-none"
+                    bare
+                  />
+                </div>
              </div>
            </div>
          ))}
--- a/src/renderer/components/team/attachments/AttachmentPreviewList.tsx
+++ b/src/renderer/components/team/attachments/AttachmentPreviewList.tsx
@ -1,4 +1,4 @@
-import { AlertCircle } from 'lucide-react';
+import { AlertCircle, X } from 'lucide-react';

 import { AttachmentPreviewItem } from './AttachmentPreviewItem';

@ -8,6 +8,7 @@ interface AttachmentPreviewListProps {
  attachments: AttachmentPayload[];
  onRemove: (id: string) => void;
  error?: string | null;
+  onDismissError?: () => void;
  /** When true, previews are overlaid with a disabled indicator (recipient doesn't support attachments). */
  disabled?: boolean;
  /** Hint text shown when disabled and attachments are present. */
@ -18,6 +19,7 @@ export const AttachmentPreviewList = ({
  attachments,
  onRemove,
  error,
+  onDismissError,
  disabled,
  disabledHint,
 }: AttachmentPreviewListProps): React.JSX.Element | null => {
@ -49,7 +51,16 @@ export const AttachmentPreviewList = ({
      {error ? (
        <div className="flex items-center gap-1.5 rounded-md bg-red-500/10 px-2.5 py-1.5">
          <AlertCircle size={13} className="shrink-0 text-red-400" />
-          <p className="text-[11px] text-red-400">{error}</p>
+          <p className="flex-1 text-[11px] text-red-400">{error}</p>
+          {onDismissError ? (
+            <button
+              type="button"
+              className="shrink-0 rounded p-0.5 text-red-400 transition-colors hover:bg-red-500/20 hover:text-red-300"
+              onClick={onDismissError}
+            >
+              <X size={12} />
+            </button>
+          ) : null}
        </div>
      ) : null}
    </div>
--- a/src/renderer/components/team/messages/MessageComposer.tsx
+++ b/src/renderer/components/team/messages/MessageComposer.tsx
@ -128,6 +128,7 @@ export const MessageComposer = ({
    addFiles,
    removeAttachment,
    clearAttachments,
+    clearError: clearAttachmentError,
    handlePaste,
    handleDrop,
  } = useAttachments({ persistenceKey: `compose:${teamName}:attachments` });
@ -408,6 +409,7 @@ export const MessageComposer = ({
        attachments={attachments}
        onRemove={removeAttachment}
        error={attachmentError}
+        onDismissError={clearAttachmentError}
        disabled={attachmentsBlocked}
        disabledHint="Image attachments are only supported when sending to the team lead while the team is online. Remove attachments or switch recipient."
      />
--- a/src/renderer/hooks/useAttachments.ts
+++ b/src/renderer/hooks/useAttachments.ts
@ -23,6 +23,7 @@ interface UseAttachmentsReturn {
  addFiles: (files: FileList | File[]) => Promise<void>;
  removeAttachment: (id: string) => void;
  clearAttachments: () => void;
+  clearError: () => void;
  handlePaste: (event: React.ClipboardEvent) => void;
  handleDrop: (event: React.DragEvent) => void;
 }
@ -219,6 +220,10 @@ export function useAttachments(options?: UseAttachmentsOptions): UseAttachmentsR
    [schedulePersist]
  );

+  const clearError = useCallback(() => {
+    setError(null);
+  }, []);
+
  const clearAttachments = useCallback(() => {
    if (timerRef.current != null) {
      clearTimeout(timerRef.current);
@ -280,6 +285,7 @@ export function useAttachments(options?: UseAttachmentsOptions): UseAttachmentsR
    addFiles,
    removeAttachment,
    clearAttachments,
+    clearError,
    handlePaste,
    handleDrop,
  };
--- a/src/renderer/index.css
+++ b/src/renderer/index.css
@ -642,6 +642,22 @@ body {
  animation: chat-message-enter 350ms ease-out both;
 }

+@keyframes thought-expand {
+  from {
+    max-height: 0;
+    opacity: 0;
+    overflow: hidden;
+  }
+  to {
+    max-height: 500px;
+    opacity: 1;
+  }
+}
+
+.thought-expand-in {
+  animation: thought-expand 350ms ease-out both;
+}
+
 .skeleton-card {
  animation: skeleton-fade-in 0.4s ease-out both;
  position: relative;
--- a/test/main/ipc/teams.test.ts
+++ b/test/main/ipc/teams.test.ts
@ -274,7 +274,6 @@ describe('ipc teams handlers', () => {
      messages: [
        {
          from: 'team-lead',
-          to: 'user',
          text: 'Hello there',
          timestamp: '2026-02-23T10:00:00.000Z',
          read: true,
@ -287,7 +286,6 @@ describe('ipc teams handlers', () => {
    provisioningService.getLiveLeadProcessMessages.mockReturnValueOnce([
      {
        from: 'team-lead',
-        to: 'user',
        text: 'Hello there',
        timestamp: '2026-02-23T10:00:01.000Z',
        read: true,
--- a/test/main/services/team/MemberStatsComputer.test.ts
+++ b/test/main/services/team/MemberStatsComputer.test.ts
@ -1,6 +1,49 @@
 import { describe, expect, it } from 'vitest';

-import { estimateBashLinesChanged } from '../../../../src/main/services/team/MemberStatsComputer';
+import {
+  estimateBashLinesChanged,
+  isValidFilePath,
+} from '../../../../src/main/services/team/MemberStatsComputer';
+
+describe('isValidFilePath', () => {
+  it('rejects null-like values', () => {
+    expect(isValidFilePath('null')).toBe(false);
+    expect(isValidFilePath('null;')).toBe(false);
+    expect(isValidFilePath('null;,')).toBe(false);
+    expect(isValidFilePath('undefined')).toBe(false);
+    expect(isValidFilePath('None')).toBe(false);
+    expect(isValidFilePath('false')).toBe(false);
+    expect(isValidFilePath('true')).toBe(false);
+    expect(isValidFilePath('')).toBe(false);
+  });
+
+  it('rejects paths without slash', () => {
+    expect(isValidFilePath('somefile')).toBe(false);
+    expect(isValidFilePath('a')).toBe(false);
+  });
+
+  it('rejects very short paths', () => {
+    expect(isValidFilePath('/')).toBe(false);
+  });
+
+  it('accepts valid file paths', () => {
+    expect(isValidFilePath('/tmp/file.txt')).toBe(true);
+    expect(isValidFilePath('/Users/dev/project/src/main.ts')).toBe(true);
+    expect(isValidFilePath('./src/index.ts')).toBe(true);
+    expect(isValidFilePath('src/utils/helper.ts')).toBe(true);
+  });
+
+  it('strips trailing punctuation before validation', () => {
+    expect(isValidFilePath('/tmp/file.txt;')).toBe(true);
+    expect(isValidFilePath('/tmp/file.txt,')).toBe(true);
+    expect(isValidFilePath('/tmp/file.txt.')).toBe(true);
+  });
+
+  it('handles whitespace', () => {
+    expect(isValidFilePath('  /tmp/file.txt  ')).toBe(true);
+    expect(isValidFilePath('  null  ')).toBe(false);
+  });
+});

 describe('estimateBashLinesChanged', () => {
  it('returns zero for simple non-writing commands', () => {