feat: enhance attachment handling and metadata management

- Added filePath to attachment metadata in various components to support file access.
- Updated saveTaskAttachmentFile and related functions to include file paths for stored attachments.
- Enhanced documentation and comments to clarify the importance of using MCP tools for task review operations.
- Improved UI components to display file paths where applicable, ensuring better user experience with attachments.

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

@@ -440,6 +440,7 @@ function saveTaskAttachmentFile(paths, taskId, flags) {
     mimeType,
     size: stats.size,
     addedAt: nowIso(),
+    filePath: destPath,
   };
 
   return {

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

@@ -393,6 +393,7 @@ function buildMemberTaskProtocol(teamName) {
    This is MANDATORY before review_approve or review_request_changes. Without this step, the kanban board may not show the task in REVIEW during your review.
 4. If you are asked to review and the task is accepted, move it to APPROVED (not DONE) with MCP tool review_approve:
    { teamName: "${teamName}", taskId: "<taskId>", note?: "<optional note>", notifyOwner: true }
+   CRITICAL: Text comments like "approved" or "LGTM" do NOT change the kanban board. You MUST call review_approve to move a task from REVIEW to APPROVED. Without the tool call the task stays stuck in the REVIEW column.
 5. If review fails and changes are needed, use MCP tool review_request_changes:
    { teamName: "${teamName}", taskId: "<taskId>", comment: "<what to fix>" }
 6. NEVER skip status updates. A task is NOT done until completed status is written.

mcp-server/src/tools/taskTools.ts

@@ -170,7 +170,7 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
         ...(source ? { source } : {}),
       };
 
-      // Preserve attachment metadata by reference only — no blob copying
+      // Preserve attachment metadata — filePath included when available
       if (Array.isArray(message.attachments) && message.attachments.length > 0) {
         sourceMessage.attachments = (message.attachments as Record<string, unknown>[])
           .filter(
@@ -185,6 +185,7 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
             filename: String(a.filename),
             mimeType: typeof a.mimeType === 'string' ? a.mimeType : '',
             size: typeof a.size === 'number' ? a.size : 0,
+            ...(typeof a.filePath === 'string' ? { filePath: a.filePath } : {}),
           }));
       }
 
@@ -212,7 +213,11 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
 
   server.addTool({
     name: 'task_get',
-    description: 'Get a task by id',
+    description:
+      'Get a task by id. Response includes:\n' +
+      '- sourceMessage.attachments: from original user message (filePath = absolute path to file on disk, use Read tool to view)\n' +
+      '- attachments: files attached to the task (filePath = absolute path, use Read tool to view)\n' +
+      '- comments[].attachments: files on comments (filePath = absolute path, use Read tool to view)',
     parameters: z.object({
       ...toolContextSchema,
       taskId: z.string().min(1),
@@ -314,7 +319,8 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
 
   server.addTool({
     name: 'task_attach_file',
-    description: 'Attach a file to a task',
+    description:
+      'Attach a file to a task. Returns attachment metadata with filePath for future access via Read tool.',
     parameters: z.object({
       ...toolContextSchema,
       taskId: z.string().min(1),
@@ -351,7 +357,8 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
 
   server.addTool({
     name: 'task_attach_comment_file',
-    description: 'Attach a file to a task comment',
+    description:
+      'Attach a file to a task comment. Returns attachment metadata with filePath for future access via Read tool.',
     parameters: z.object({
       ...toolContextSchema,
       taskId: z.string().min(1),

src/main/ipc/teams.ts

@@ -1247,12 +1247,30 @@ async function handleSendMessage(
       }
 
       if (stdinSent) {
-        const attachmentMeta: AttachmentMeta[] | undefined = validatedAttachments?.map((a) => ({
-          id: a.id,
-          filename: a.filename,
-          mimeType: a.mimeType,
-          size: a.size,
-        }));
+        // Save attachment files to disk FIRST to get file paths for metadata
+        let attachmentFilePaths: Map<string, string> | undefined;
+        if (validatedAttachments?.length) {
+          try {
+            attachmentFilePaths = await attachmentStore.saveAttachments(
+              tn,
+              preGeneratedMessageId,
+              validatedAttachments
+            );
+          } catch (e) {
+            logger.warn(`Failed to save attachments: ${e}`);
+          }
+        }
+
+        const attachmentMeta: AttachmentMeta[] | undefined = validatedAttachments?.map((a) => {
+          const fp = attachmentFilePaths?.get(a.id);
+          return {
+            id: a.id,
+            filename: a.filename,
+            mimeType: a.mimeType,
+            size: a.size,
+            ...(fp ? { filePath: fp } : {}),
+          };
+        });
 
         // Persistence is best-effort — stdin already delivered the message
         let result: SendMessageResult;
@@ -1271,12 +1289,7 @@ async function handleSendMessage(
           result = { deliveredToInbox: false, messageId: preGeneratedMessageId };
         }
 
-        // Save attachment binary data to disk (best-effort)
-        if (validatedAttachments?.length && result.messageId) {
-          void attachmentStore
-            .saveAttachments(tn, result.messageId, validatedAttachments)
-            .catch((e) => logger.warn(`Failed to save attachments: ${e}`));
-        }
+        // Attachment files already saved above (before metadata construction)
 
         provisioning.pushLiveLeadProcessMessage(tn, {
           from: 'user',

src/main/services/team/TeamAttachmentStore.ts

@@ -3,18 +3,27 @@ import { createLogger } from '@shared/utils/logger';
 import * as fs from 'fs';
 import * as path from 'path';
 
-import { atomicWriteAsync } from './atomicWrite';
-
 import type { AttachmentFileData, AttachmentPayload } from '@shared/types';
 
 const logger = createLogger('Service:TeamAttachmentStore');
 
-const MAX_ATTACHMENTS_FILE_BYTES = 64 * 1024 * 1024; // 64MB safety cap
+const MAX_ATTACHMENT_SIZE = 20 * 1024 * 1024; // 20 MB per file
+const MAX_ATTACHMENTS_FILE_BYTES = 64 * 1024 * 1024; // 64MB legacy JSON cap
+
+/** Per-attachment metadata stored in the index file. */
+interface StoredAttachmentIndex {
+  id: string;
+  filename: string;
+  mimeType: string;
+}
 
 export class TeamAttachmentStore {
   private assertSafePathSegment(label: string, value: string): void {
     if (
       value.length === 0 ||
+      value.trim().length === 0 ||
+      value === '.' ||
+      value === '..' ||
       value.includes('/') ||
       value.includes('\\') ||
       value.includes('..') ||
@@ -24,37 +33,204 @@ export class TeamAttachmentStore {
     }
   }
 
-  private getDir(teamName: string): string {
+  private sanitizeStoredFilename(original: string): string {
+    const raw = String(original ?? '').trim();
+    const base = raw ? (raw.split(/[\\/]/).pop() ?? raw) : '';
+    const cleaned = base
+      .replace(/\0/g, '')
+      .replace(/[\r\n\t]/g, ' ')
+      .replace(/[\\/]/g, '_')
+      .trim();
+    if (!cleaned) return 'attachment';
+    return cleaned.length > 180 ? cleaned.slice(0, 180) : cleaned;
+  }
+
+  /** Base directory for all message attachments of a team. */
+  private getTeamDir(teamName: string): string {
     this.assertSafePathSegment('teamName', teamName);
     return path.join(getAppDataPath(), 'attachments', teamName);
   }
 
-  private getFilePath(teamName: string, messageId: string): string {
+  /** Directory for a specific message's attachments (new file-based format). */
+  private getMessageDir(teamName: string, messageId: string): string {
     this.assertSafePathSegment('messageId', messageId);
-    return path.join(this.getDir(teamName), `${messageId}.json`);
+    return path.join(this.getTeamDir(teamName), messageId);
   }
 
+  /** Path to the metadata index file inside a message attachment directory. */
+  private getIndexPath(teamName: string, messageId: string): string {
+    return path.join(this.getMessageDir(teamName, messageId), '_index.json');
+  }
+
+  /** Legacy JSON bundle path (old format). */
+  private getLegacyFilePath(teamName: string, messageId: string): string {
+    this.assertSafePathSegment('messageId', messageId);
+    return path.join(this.getTeamDir(teamName), `${messageId}.json`);
+  }
+
+  /** Stored file path for an individual attachment. */
+  private getStoredFilePath(
+    teamName: string,
+    messageId: string,
+    attachmentId: string,
+    filename: string
+  ): string {
+    this.assertSafePathSegment('attachmentId', attachmentId);
+    const safeName = this.sanitizeStoredFilename(filename);
+    return path.join(this.getMessageDir(teamName, messageId), `${attachmentId}--${safeName}`);
+  }
+
+  /**
+   * Save message attachments as individual files on disk.
+   * Returns a Map of attachmentId → absolute file path for each saved file.
+   */
   async saveAttachments(
     teamName: string,
     messageId: string,
     attachments: AttachmentPayload[]
-  ): Promise<void> {
-    if (attachments.length === 0) return;
+  ): Promise<Map<string, string>> {
+    const filePaths = new Map<string, string>();
+    if (attachments.length === 0) return filePaths;
 
-    const fileData: AttachmentFileData[] = attachments.map((a) => ({
-      id: a.id,
-      data: a.data,
-      mimeType: a.mimeType,
-    }));
+    const dir = this.getMessageDir(teamName, messageId);
+    await fs.promises.mkdir(dir, { recursive: true });
+
+    const indexEntries: StoredAttachmentIndex[] = [];
+
+    for (const att of attachments) {
+      const buffer = Buffer.from(att.data, 'base64');
+      if (buffer.length > MAX_ATTACHMENT_SIZE) {
+        logger.warn(
+          `[${teamName}] Skipping oversized attachment ${att.id} (${(buffer.length / (1024 * 1024)).toFixed(1)} MB)`
+        );
+        continue;
+      }
+
+      const storedPath = this.getStoredFilePath(teamName, messageId, att.id, att.filename);
+      try {
+        await fs.promises.writeFile(storedPath, buffer);
+      } catch (writeError) {
+        logger.warn(`[${teamName}] Failed to write attachment ${att.id}: ${writeError}`);
+        continue;
+      }
+      filePaths.set(att.id, storedPath);
+
+      indexEntries.push({
+        id: att.id,
+        filename: att.filename,
+        mimeType: att.mimeType,
+      });
+    }
+
+    // Write metadata index for successful files (mimeType, original filename)
+    if (indexEntries.length > 0) {
+      const indexPath = this.getIndexPath(teamName, messageId);
+      await fs.promises.writeFile(indexPath, JSON.stringify(indexEntries, null, 2));
+    }
 
-    await atomicWriteAsync(this.getFilePath(teamName, messageId), JSON.stringify(fileData));
     logger.debug(
-      `[${teamName}] Saved ${attachments.length} attachment(s) for message ${messageId}`
+      `[${teamName}] Saved ${filePaths.size} attachment file(s) for message ${messageId}`
     );
+    return filePaths;
   }
 
+  /**
+   * Returns a Map of attachmentId → absolute file path.
+   * Only available for new file-based format. Legacy JSON bundles have no individual file paths.
+   */
+  async getAttachmentFilePaths(teamName: string, messageId: string): Promise<Map<string, string>> {
+    const result = new Map<string, string>();
+
+    // Try new file-based format first
+    const dir = this.getMessageDir(teamName, messageId);
+    try {
+      const entries = await fs.promises.readdir(dir);
+      for (const entry of entries) {
+        if (entry === '_index.json') continue;
+        const dashIdx = entry.indexOf('--');
+        if (dashIdx > 0) {
+          const attachmentId = entry.slice(0, dashIdx);
+          result.set(attachmentId, path.join(dir, entry));
+        }
+      }
+      if (result.size > 0) return result;
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
+    }
+
+    // No new-format files found — not available for legacy format
+    return result;
+  }
+
+  /**
+   * Read attachment data (base64) for rendering in UI.
+   * Supports both new file-based format and legacy JSON bundle.
+   */
   async getAttachments(teamName: string, messageId: string): Promise<AttachmentFileData[]> {
-    const filePath = this.getFilePath(teamName, messageId);
+    // Try new file-based format first
+    const newResult = await this.readNewFormatAttachments(teamName, messageId);
+    if (newResult !== null) return newResult;
+
+    // Fallback to legacy JSON format
+    return this.readLegacyAttachments(teamName, messageId);
+  }
+
+  /** Read attachments from new file-based directory format. */
+  private async readNewFormatAttachments(
+    teamName: string,
+    messageId: string
+  ): Promise<AttachmentFileData[] | null> {
+    const indexPath = this.getIndexPath(teamName, messageId);
+
+    let indexRaw: string;
+    try {
+      indexRaw = await fs.promises.readFile(indexPath, 'utf8');
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null;
+      throw error;
+    }
+
+    let index: StoredAttachmentIndex[];
+    try {
+      const parsed = JSON.parse(indexRaw) as unknown;
+      if (!Array.isArray(parsed)) return null;
+      index = parsed as StoredAttachmentIndex[];
+    } catch {
+      return null;
+    }
+
+    const result: AttachmentFileData[] = [];
+    for (const entry of index) {
+      if (!entry || typeof entry.id !== 'string') continue;
+      const filename =
+        typeof entry.filename === 'string' && entry.filename ? entry.filename : 'attachment';
+      const mimeType =
+        typeof entry.mimeType === 'string' && entry.mimeType
+          ? entry.mimeType
+          : 'application/octet-stream';
+      const filePath = this.getStoredFilePath(teamName, messageId, entry.id, filename);
+      try {
+        const buffer = await fs.promises.readFile(filePath);
+        result.push({
+          id: entry.id,
+          data: buffer.toString('base64'),
+          mimeType,
+        });
+      } catch (error) {
+        if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue;
+        throw error;
+      }
+    }
+
+    return result;
+  }
+
+  /** Read attachments from legacy JSON bundle format. */
+  private async readLegacyAttachments(
+    teamName: string,
+    messageId: string
+  ): Promise<AttachmentFileData[]> {
+    const filePath = this.getLegacyFilePath(teamName, messageId);
 
     let raw: string;
     try {
@@ -103,5 +279,5 @@ export class TeamAttachmentStore {
   }
 
   // TODO: add deleteAttachments(teamName, messageId) for cleanup on failed/cancelled sends.
-  // Best-effort removal of the attachment JSON file — useful for retry/cancel flows.
+  // Best-effort removal of attachment files — useful for retry/cancel flows.
 }

src/main/services/team/TeamProvisioningService.ts

@@ -558,6 +558,13 @@ function buildTeamCtlOpsInstructions(teamName: string, leadName: string): string
       `- Link related: task_link { teamName: "${teamName}", taskId: "<id>", targetId: "<targetId>", relationship: "related" }`,
       `- Unlink: task_unlink { teamName: "${teamName}", taskId: "<id>", targetId: "<targetId>", relationship: "blocked-by" }`,
       ``,
+      `Review operations — use MCP tools directly (text comments do NOT change kanban state):`,
+      `- Request review (after task_complete): review_request { teamName: "${teamName}", taskId: "<id>", from: "${leadName}", reviewer: "<reviewer-name>" }`,
+      `- Start review (reviewer signals they are beginning): review_start { teamName: "${teamName}", taskId: "<id>", from: "<reviewer-name>" }`,
+      `- Approve review: review_approve { teamName: "${teamName}", taskId: "<id>", note?: "<note>", notifyOwner: true }`,
+      `- Request changes: review_request_changes { teamName: "${teamName}", taskId: "<id>", comment: "<what to fix>" }`,
+      `CRITICAL: Writing "approved" or "LGTM" as a task comment does NOT move the task on the kanban board. You MUST call the review_approve MCP tool. Without the tool call the task stays stuck in the REVIEW column.`,
+      ``,
       `Attachment storage modes (IMPORTANT):`,
       `- Default is copy (safe, robust).`,
       `- Use mode: "link" to try a hardlink (no duplication). It may fall back to copy unless you disable fallback.`,
@@ -831,6 +838,7 @@ function buildProvisioningPrompt(request: TeamCreateRequest): string {
   - If a task is blocked (uses blockedBy), it MUST be created as pending (for example with task_create + startImmediately: false). Do NOT mark blocked tasks in_progress.
      - Review guidance:
       - Prefer NOT creating a separate "review task". Our workflow reviews the work task itself: call review_start when beginning review, then review_approve/review_request_changes on the implementation task #X.
+      - CRITICAL: Text comments ("approved", "LGTM") do NOT move the task on the kanban board. You MUST call the MCP tool review_approve to move from REVIEW to APPROVED. Without the tool call, the task stays stuck in REVIEW.
        - If you MUST create a separate review reminder/assignment task, create it as pending and link it to the work task:
         - Use related to connect it to #X (non-blocking link).
         - If the review truly cannot start until #X is done, ALSO add blockedBy #X.

src/main/services/team/TeamTaskAttachmentStore.ts

@@ -124,6 +124,7 @@ export class TeamTaskAttachmentStore {
       mimeType,
       size: buffer.length,
       addedAt: new Date().toISOString(),
+      filePath,
     };
 
     logger.debug(`[${teamName}] Saved task attachment ${attachmentId} for task #${taskId}`);

src/renderer/components/team/attachments/SourceMessageAttachments.tsx

@@ -23,6 +23,7 @@ export const SourceMessageAttachments = ({
     filename: a.filename,
     mimeType: a.mimeType,
     size: a.size,
+    ...(a.filePath ? { filePath: a.filePath } : {}),
   }));
 
   const truncatedText =

src/renderer/components/team/messages/MessagesPanel.tsx

@@ -9,7 +9,6 @@ import { useTeamMessagesRead } from '@renderer/hooks/useTeamMessagesRead';
 import { useStore } from '@renderer/store';
 import { filterTeamMessages } from '@renderer/utils/teamMessageFiltering';
 import { toMessageKey } from '@renderer/utils/teamMessageKey';
-import { stripAgentBlocks } from '@shared/constants/agentBlocks';
 import {
   Bell,
   CheckCheck,
@@ -321,7 +320,7 @@ export const MessagesPanel = memo(function MessagesPanel({
   );
 
   const messagesContent = (
-    <>
+    <div className="pb-14">
       <MessageComposer
         teamName={teamName}
         members={members}
@@ -381,7 +380,7 @@ export const MessagesPanel = memo(function MessagesPanel({
         teamColorByName={teamColorByName}
         onTeamClick={openTeamTab}
       />
-    </>
+    </div>
   );
 
   // ---- Sidebar mode ----
@@ -497,7 +496,7 @@ export const MessagesPanel = memo(function MessagesPanel({
           </div>
         )}
         {/* Scrollable content */}
-        <div className="min-h-0 min-w-0 flex-1 overflow-y-auto overflow-x-hidden py-2 pr-3">
+        <div className="min-h-0 min-w-0 flex-1 overflow-y-auto overflow-x-hidden pb-14 pr-3 pt-2">
           <div className="pl-3">
             <MessageComposer
               teamName={teamName}

src/renderer/constants/teamColors.ts

@@ -88,12 +88,12 @@ const TEAMMATE_COLORS: Record<string, TeamColorSet> = {
   },
   /** Reserved for the human user — never assigned to team members. */
   user: {
-    border: '#f5f5f4',
-    borderLight: '#78716c',
-    badge: 'rgba(245, 245, 244, 0.12)',
-    badgeLight: 'rgba(87, 83, 78, 0.18)',
+    border: '#a8a29e',
+    borderLight: '#57534e',
+    badge: 'rgba(168, 162, 158, 0.18)',
+    badgeLight: 'rgba(68, 64, 60, 0.14)',
     text: '#d6d3d1',
-    textLight: '#44403c',
+    textLight: '#292524',
   },
 };
 

src/shared/types/team.ts

@@ -154,8 +154,14 @@ export interface SourceMessageSnapshot {
   timestamp: string;
   /** Message source type (e.g. "user_sent", "inbox"). */
   source?: string;
-  /** Attachment metadata references (IDs only, no blobs). */
-  attachments?: { id: string; filename: string; mimeType: string; size: number }[];
+  /** Attachment metadata references (IDs only, no blobs). filePath present when file is stored on disk. */
+  attachments?: {
+    id: string;
+    filename: string;
+    mimeType: string;
+    size: number;
+    filePath?: string;
+  }[];
 }
 
 // Fields are validated in TeamTaskReader.getTasks() using `satisfies Record<keyof TeamTask, unknown>`.
@@ -229,6 +235,8 @@ export interface TaskAttachmentMeta {
   size: number;
   /** ISO timestamp when the attachment was added. */
   addedAt: string;
+  /** Absolute path to the file on disk. Null/absent for metadata-only references. */
+  filePath?: string | null;
 }
 
 /** Payload for uploading an attachment with base64 data (renderer → main). */
@@ -256,6 +264,8 @@ export interface AttachmentMeta {
   filename: string;
   mimeType: AttachmentMediaType;
   size: number;
+  /** Absolute path to the file on disk. Absent for metadata-only references. */
+  filePath?: string;
 }
 
 export interface AttachmentPayload extends AttachmentMeta {