From 95d610f43b1323e4105be087f469b69e253907d4 Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 7 Mar 2026 17:47:28 +0200 Subject: [PATCH] feat: enhance review and messaging functionalities - Added requestReview method to facilitate task review requests, integrating with kanban and messaging systems. - Updated taskStore to manage review states, ensuring compatibility with kanban columns. - Enhanced message handling by introducing appendSentMessage for better tracking of sent messages. - Improved task management by normalizing review states and integrating them into task creation and retrieval processes. - Updated tests to cover new review and messaging features, ensuring robust functionality across components. --- README.md | 2 +- .../src/internal/kanbanStore.js | 9 + .../src/internal/messageStore.js | 144 ++++++++++++++++ .../src/internal/messages.js | 9 +- .../src/internal/processStore.js | 17 +- agent-teams-controller/src/internal/review.js | 52 ++++++ .../src/internal/taskStore.js | 47 ++++- .../test/controller.test.js | 48 +++++- mcp-server/src/agent-teams-controller.d.ts | 2 + mcp-server/src/tools/messageTools.ts | 27 ++- mcp-server/src/tools/reviewTools.ts | 18 ++ mcp-server/test/tools.test.ts | 45 +++++ src/main/index.ts | 14 ++ src/main/services/team/TeamDataService.ts | 160 ++++++++---------- src/main/services/team/TeamInboxReader.ts | 15 ++ .../services/team/TeamProvisioningService.ts | 42 +++-- src/main/services/team/TeamTaskReader.ts | 3 + src/main/workers/team-fs-worker.ts | 8 + .../sections/NotificationsSection.tsx | 6 +- .../components/sidebar/SidebarTaskItem.tsx | 6 +- .../components/sidebar/taskFiltersState.ts | 18 +- src/renderer/components/team/TaskTooltip.tsx | 4 +- .../team/dialogs/CreateTaskDialog.tsx | 5 +- .../team/dialogs/TaskDetailDialog.tsx | 10 +- .../components/team/kanban/KanbanBoard.tsx | 10 +- .../team/members/MemberTasksTab.tsx | 3 +- .../components/team/tasks/TaskRow.tsx | 6 +- src/renderer/store/slices/teamSlice.ts | 7 +- src/shared/types/team.ts | 3 + src/shared/utils/reviewState.ts | 42 +++++ src/types/agent-teams-controller.d.ts | 2 + .../services/team/TeamDataService.test.ts | 96 ++++++++++- .../team/TeamProvisioningServiceRelay.test.ts | 27 ++- 33 files changed, 750 insertions(+), 157 deletions(-) create mode 100644 agent-teams-controller/src/internal/messageStore.js create mode 100644 src/shared/utils/reviewState.ts diff --git a/README.md b/README.md index 37712043..63997838 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@

- 100% free, open source. No API keys. No configuration. Runs entirely locally. + 100% free, open source. No API keys. No configuration. Runs entirely locally. Not just coding agents.


diff --git a/agent-teams-controller/src/internal/kanbanStore.js b/agent-teams-controller/src/internal/kanbanStore.js index 69de480f..e6f70608 100644 --- a/agent-teams-controller/src/internal/kanbanStore.js +++ b/agent-teams-controller/src/internal/kanbanStore.js @@ -1,5 +1,6 @@ const fs = require('fs'); const path = require('path'); +const taskStore = require('./taskStore.js'); function nowIso() { return new Date().toISOString(); @@ -80,6 +81,10 @@ function setKanbanColumn(paths, teamName, taskId, column) { ? { column: 'review', reviewer: null, movedAt: nowIso() } : { column: 'approved', movedAt: nowIso() }; writeKanbanState(paths, teamName, state); + taskStore.updateTask(paths, String(taskId), (task) => ({ + ...task, + reviewState: column, + })); return state; } @@ -87,6 +92,10 @@ function clearKanban(paths, teamName, taskId) { const state = readKanbanState(paths, teamName); delete state.tasks[String(taskId)]; writeKanbanState(paths, teamName, state); + taskStore.updateTask(paths, String(taskId), (task) => ({ + ...task, + reviewState: 'none', + })); return state; } diff --git a/agent-teams-controller/src/internal/messageStore.js b/agent-teams-controller/src/internal/messageStore.js new file mode 100644 index 00000000..636e1bef --- /dev/null +++ b/agent-teams-controller/src/internal/messageStore.js @@ -0,0 +1,144 @@ +const fs = require('fs'); +const path = require('path'); +const crypto = require('crypto'); + +function nowIso() { + return new Date().toISOString(); +} + +function ensureDir(dirPath) { + fs.mkdirSync(dirPath, { recursive: true }); +} + +function readJson(filePath, fallbackValue) { + try { + return JSON.parse(fs.readFileSync(filePath, 'utf8')); + } catch { + return fallbackValue; + } +} + +function writeJson(filePath, value) { + ensureDir(path.dirname(filePath)); + const tempPath = `${filePath}.${process.pid}.${Date.now()}.tmp`; + fs.writeFileSync(tempPath, JSON.stringify(value, null, 2)); + fs.renameSync(tempPath, filePath); +} + +function getInboxPath(paths, memberName) { + return path.join(paths.teamDir, 'inboxes', `${String(memberName).trim()}.json`); +} + +function getSentMessagesPath(paths) { + return path.join(paths.teamDir, 'sentMessages.json'); +} + +function normalizeAttachments(attachments) { + if (!Array.isArray(attachments) || attachments.length === 0) { + return undefined; + } + + const normalized = attachments + .filter((item) => item && typeof item === 'object') + .map((item) => ({ + id: String(item.id || '').trim(), + filename: String(item.filename || '').trim(), + mimeType: String(item.mimeType || '').trim(), + size: Number(item.size || 0), + })) + .filter((item) => item.id && item.filename && item.mimeType && Number.isFinite(item.size)); + + return normalized.length > 0 ? normalized : undefined; +} + +function buildMessage(flags, defaults) { + const timestamp = + typeof flags.timestamp === 'string' && flags.timestamp.trim() ? flags.timestamp.trim() : nowIso(); + const messageId = + typeof flags.messageId === 'string' && flags.messageId.trim() + ? flags.messageId.trim() + : crypto.randomUUID(); + const attachments = normalizeAttachments(flags.attachments); + + return { + from: + typeof flags.from === 'string' && flags.from.trim() + ? flags.from.trim() + : defaults.from || 'user', + ...(defaults.to ? { to: defaults.to } : {}), + text: String(flags.text || ''), + timestamp, + read: defaults.read, + ...(typeof flags.summary === 'string' && flags.summary.trim() + ? { summary: flags.summary.trim() } + : {}), + ...(typeof flags.source === 'string' && flags.source.trim() ? { source: flags.source.trim() } : {}), + ...(typeof flags.leadSessionId === 'string' && flags.leadSessionId.trim() + ? { leadSessionId: flags.leadSessionId.trim() } + : {}), + ...(typeof flags.color === 'string' && flags.color.trim() ? { color: flags.color.trim() } : {}), + ...(typeof flags.toolSummary === 'string' && flags.toolSummary.trim() + ? { toolSummary: flags.toolSummary.trim() } + : {}), + ...(Array.isArray(flags.toolCalls) && flags.toolCalls.length > 0 + ? { + toolCalls: flags.toolCalls + .filter((item) => item && typeof item === 'object' && typeof item.name === 'string') + .map((item) => ({ + name: item.name, + ...(typeof item.preview === 'string' ? { preview: item.preview } : {}), + })), + } + : {}), + ...(attachments ? { attachments } : {}), + messageId, + }; +} + +function appendRow(filePath, row) { + const current = readJson(filePath, []); + const list = Array.isArray(current) ? current : []; + list.push(row); + writeJson(filePath, list); + return row; +} + +function sendInboxMessage(paths, flags) { + const memberName = + typeof flags.member === 'string' && flags.member.trim() + ? flags.member.trim() + : typeof flags.to === 'string' && flags.to.trim() + ? flags.to.trim() + : ''; + if (!memberName) { + throw new Error('Missing recipient'); + } + + const payload = buildMessage(flags, { + from: 'user', + to: memberName, + read: false, + }); + appendRow(getInboxPath(paths, memberName), payload); + return { + deliveredToInbox: true, + messageId: payload.messageId, + message: payload, + }; +} + +function appendSentMessage(paths, flags) { + const payload = buildMessage(flags, { + from: 'team-lead', + to: typeof flags.to === 'string' && flags.to.trim() ? flags.to.trim() : undefined, + read: true, + }); + appendRow(getSentMessagesPath(paths), payload); + return payload; +} + +module.exports = { + appendSentMessage, + sendInboxMessage, +}; + diff --git a/agent-teams-controller/src/internal/messages.js b/agent-teams-controller/src/internal/messages.js index f13d94ee..c2101a77 100644 --- a/agent-teams-controller/src/internal/messages.js +++ b/agent-teams-controller/src/internal/messages.js @@ -1,9 +1,14 @@ -const legacy = require('../legacy/teamctl.cli.js'); +const messageStore = require('./messageStore.js'); function sendMessage(context, flags) { - return legacy.sendInboxMessage(context.paths, context.teamName, flags); + return messageStore.sendInboxMessage(context.paths, flags); +} + +function appendSentMessage(context, flags) { + return messageStore.appendSentMessage(context.paths, flags); } module.exports = { + appendSentMessage, sendMessage, }; diff --git a/agent-teams-controller/src/internal/processStore.js b/agent-teams-controller/src/internal/processStore.js index 2455bf12..b65d4cb0 100644 --- a/agent-teams-controller/src/internal/processStore.js +++ b/agent-teams-controller/src/internal/processStore.js @@ -79,10 +79,19 @@ function registerProcess(paths, flags) { const list = readProcesses(paths); const existingActiveIndex = list.findIndex((entry) => entry.pid === pid && !entry.stoppedAt); - const existingActive = existingActiveIndex >= 0 ? list[existingActiveIndex] : null; + const existingActive = + existingActiveIndex >= 0 + ? { + ...list[existingActiveIndex], + ...(legacy.isProcessAlive(pid) ? {} : { stoppedAt: nowIso() }), + } + : null; + if (existingActiveIndex >= 0 && existingActive && existingActive.stoppedAt) { + list[existingActiveIndex] = existingActive; + } const now = nowIso(); const entry = { - id: existingActive ? existingActive.id : crypto.randomUUID(), + id: existingActive && !existingActive.stoppedAt ? existingActive.id : crypto.randomUUID(), label, pid, ...(flags.port != null ? { port: Number(flags.port) } : {}), @@ -94,10 +103,10 @@ function registerProcess(paths, flags) { ...(typeof flags.command === 'string' && flags.command.trim() ? { command: flags.command.trim() } : {}), - registeredAt: existingActive ? existingActive.registeredAt : now, + registeredAt: existingActive && !existingActive.stoppedAt ? existingActive.registeredAt : now, }; - if (existingActiveIndex >= 0) { + if (existingActiveIndex >= 0 && existingActive && !existingActive.stoppedAt) { list[existingActiveIndex] = entry; } else { list.push(entry); diff --git a/agent-teams-controller/src/internal/review.js b/agent-teams-controller/src/internal/review.js index e3875589..6ce6fcb8 100644 --- a/agent-teams-controller/src/internal/review.js +++ b/agent-teams-controller/src/internal/review.js @@ -2,6 +2,57 @@ const kanban = require('./kanban.js'); const messages = require('./messages.js'); const tasks = require('./tasks.js'); +function getReviewer(context, flags) { + if (typeof flags.reviewer === 'string' && flags.reviewer.trim()) { + return flags.reviewer.trim(); + } + const state = kanban.getKanbanState(context); + return typeof state.reviewers[0] === 'string' && state.reviewers[0].trim() + ? state.reviewers[0].trim() + : null; +} + +function requestReview(context, taskId, flags = {}) { + const task = tasks.getTask(context, taskId); + if (task.status !== 'completed') { + throw new Error(`Task #${task.displayId || task.id} must be completed before review`); + } + + const from = + typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'team-lead'; + const reviewer = getReviewer(context, flags); + + try { + kanban.setKanbanColumn(context, task.id, 'review'); + if (!reviewer) { + return tasks.getTask(context, task.id); + } + + messages.sendMessage(context, { + to: reviewer, + from, + text: + `Please review task #${task.displayId || task.id}.\n\n` + + '\n' + + `When approved, use MCP tool review_approve:\n` + + `{ teamName: "${context.teamName}", taskId: "${task.id}", notifyOwner: true }\n\n` + + `If changes are needed, use MCP tool review_request_changes:\n` + + `{ teamName: "${context.teamName}", taskId: "${task.id}", comment: "..." }\n` + + '', + summary: `Review request for #${task.displayId || task.id}`, + source: 'system_notification', + }); + return tasks.getTask(context, task.id); + } catch (error) { + try { + kanban.clearKanban(context, task.id); + } catch { + // Best-effort rollback: keep the original error. + } + throw error; + } +} + function approveReview(context, taskId, flags = {}) { const task = tasks.getTask(context, taskId); const from = @@ -66,5 +117,6 @@ function requestChanges(context, taskId, flags = {}) { module.exports = { approveReview, + requestReview, requestChanges, }; diff --git a/agent-teams-controller/src/internal/taskStore.js b/agent-teams-controller/src/internal/taskStore.js index af472723..bc0c372f 100644 --- a/agent-teams-controller/src/internal/taskStore.js +++ b/agent-teams-controller/src/internal/taskStore.js @@ -3,6 +3,7 @@ const path = require('path'); const crypto = require('crypto'); const TASK_STATUSES = new Set(['pending', 'in_progress', 'completed', 'deleted']); +const REVIEW_STATES = new Set(['none', 'review', 'approved']); const UUID_TASK_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; @@ -61,15 +62,43 @@ function normalizeTask(rawTask, filePath) { typeof rawTask.displayId === 'string' && rawTask.displayId.trim() ? rawTask.displayId.trim() : deriveDisplayId(id), + reviewState: normalizeTaskReviewState(rawTask.reviewState), }; return task; } +function normalizeTaskReviewState(value) { + return REVIEW_STATES.has(String(value || '').trim()) ? String(value).trim() : 'none'; +} + +function getOverlayReviewState(overlayTasks, taskId) { + if (!overlayTasks || typeof overlayTasks !== 'object') { + return 'none'; + } + + const entry = overlayTasks[String(taskId)]; + if (!entry || typeof entry !== 'object') { + return 'none'; + } + + return entry.column === 'review' || entry.column === 'approved' ? entry.column : 'none'; +} + +function withCompatibleReviewState(task, overlayTasks) { + const explicit = normalizeTaskReviewState(task.reviewState); + return explicit === 'none' ? { ...task, reviewState: getOverlayReviewState(overlayTasks, task.id) } : task; +} + function listRawTasks(paths) { ensureDir(paths.tasksDir); const entries = fs.readdirSync(paths.tasksDir); const out = []; + const overlayState = readJson(paths.kanbanPath, null); + const overlayTasks = + overlayState && typeof overlayState === 'object' && overlayState.tasks && typeof overlayState.tasks === 'object' + ? overlayState.tasks + : null; for (const fileName of entries) { if (!fileName.endsWith('.json') || fileName.startsWith('.')) continue; @@ -78,7 +107,7 @@ function listRawTasks(paths) { if (!rawTask) continue; if (rawTask.metadata && rawTask.metadata._internal === true) continue; try { - out.push(normalizeTask(rawTask, filePath)); + out.push(withCompatibleReviewState(normalizeTask(rawTask, filePath), overlayTasks)); } catch { // Skip unreadable task rows. } @@ -136,7 +165,12 @@ function readTask(paths, taskRef, options = {}) { if (!rawTask) { throw new Error(`Task not found: ${String(taskRef)}`); } - return normalizeTask(rawTask, taskPath); + const overlayState = readJson(paths.kanbanPath, null); + const overlayTasks = + overlayState && typeof overlayState === 'object' && overlayState.tasks && typeof overlayState.tasks === 'object' + ? overlayState.tasks + : null; + return withCompatibleReviewState(normalizeTask(rawTask, taskPath), overlayTasks); } function createStatusTransition(history, from, to, actor, timestamp) { @@ -291,6 +325,7 @@ function createTask(paths, input = {}) { input.needsClarification === 'lead' || input.needsClarification === 'user' ? input.needsClarification : undefined, + reviewState: normalizeTaskReviewState(input.reviewState), deletedAt: status === 'deleted' && typeof input.deletedAt === 'string' ? input.deletedAt : undefined, attachments: Array.isArray(input.attachments) ? input.attachments : undefined, @@ -591,8 +626,14 @@ function formatTaskBriefing(paths, teamName, memberName) { for (const task of activeTasks) { const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; const reviewState = kanbanEntry && kanbanEntry.column ? `, review=${kanbanEntry.column}` : ''; + const effectiveReviewState = + normalizeTaskReviewState(task.reviewState) !== 'none' + ? normalizeTaskReviewState(task.reviewState) + : reviewState + ? String(kanbanEntry.column) + : 'none'; lines.push( - `${buildTaskReference(task)} [status=${task.status}${reviewState}] ${task.subject}` + `${buildTaskReference(task)} [status=${task.status}${effectiveReviewState !== 'none' ? `, review=${effectiveReviewState}` : ''}] ${task.subject}` ); if (task.description) lines.push(` Description: ${task.description}`); if (task.blockedBy && task.blockedBy.length > 0) { diff --git a/agent-teams-controller/test/controller.test.js b/agent-teams-controller/test/controller.test.js index 9049b5ea..aa4a461d 100644 --- a/agent-teams-controller/test/controller.test.js +++ b/agent-teams-controller/test/controller.test.js @@ -44,16 +44,29 @@ describe('agent-teams-controller API', () => { ); expect(created.displayId).toHaveLength(8); expect(created.status).toBe('pending'); + expect(created.reviewState).toBe('none'); expect(controller.tasks.getTask(base.id).blocks).toEqual([created.id]); expect(controller.tasks.getTask(created.displayId).blockedBy).toEqual([base.id, dependency.id]); controller.kanban.addReviewer('alice'); - controller.kanban.setKanbanColumn(created.id, 'review'); + controller.tasks.completeTask(created.id, 'bob'); + controller.review.requestReview(created.id, { from: 'alice' }); controller.review.approveReview(created.id, { 'notify-owner': true, from: 'alice' }); const kanbanState = controller.kanban.getKanbanState(); expect(kanbanState.reviewers).toEqual(['alice']); expect(kanbanState.tasks[created.id].column).toBe('approved'); + expect(controller.tasks.getTask(created.id).reviewState).toBe('approved'); + + const sent = controller.messages.appendSentMessage({ + from: 'team-lead', + to: 'user', + text: 'All good', + leadSessionId: 'session-1', + source: 'lead_process', + attachments: [{ id: 'a1', filename: 'diff.txt', mimeType: 'text/plain', size: 12 }], + }); + expect(sent.leadSessionId).toBe('session-1'); const proc = controller.processes.registerProcess({ pid: process.pid, @@ -65,4 +78,37 @@ describe('agent-teams-controller API', () => { const stopped = controller.processes.stopProcess({ pid: process.pid }); expect(typeof stopped.stoppedAt).toBe('string'); }); + + it('creates a fresh registry entry when an old pid was recycled without stoppedAt', () => { + const claudeDir = makeClaudeDir(); + const controller = createController({ teamName: 'my-team', claudeDir }); + const processesPath = path.join(claudeDir, 'teams', 'my-team', 'processes.json'); + + fs.writeFileSync( + processesPath, + JSON.stringify( + [ + { + id: 'old-entry', + pid: 999999, + label: 'stale', + registeredAt: '2024-01-01T00:00:00.000Z', + }, + ], + null, + 2 + ) + ); + + const registered = controller.processes.registerProcess({ + pid: 999999, + label: 'fresh', + }); + + expect(registered.id).not.toBe('old-entry'); + const rows = JSON.parse(fs.readFileSync(processesPath, 'utf8')); + expect(rows).toHaveLength(2); + expect(rows[0].stoppedAt).toBeTruthy(); + expect(rows[1].id).toBe(registered.id); + }); }); diff --git a/mcp-server/src/agent-teams-controller.d.ts b/mcp-server/src/agent-teams-controller.d.ts index 71c99de1..07f85bb6 100644 --- a/mcp-server/src/agent-teams-controller.d.ts +++ b/mcp-server/src/agent-teams-controller.d.ts @@ -39,11 +39,13 @@ declare module 'agent-teams-controller' { } export interface ControllerReviewApi { + requestReview(taskId: string, flags?: Record): unknown; approveReview(taskId: string, flags?: Record): unknown; requestChanges(taskId: string, flags?: Record): unknown; } export interface ControllerMessageApi { + appendSentMessage(flags: Record): unknown; sendMessage(flags: Record): unknown; } diff --git a/mcp-server/src/tools/messageTools.ts b/mcp-server/src/tools/messageTools.ts index 5052489c..c0c1f97e 100644 --- a/mcp-server/src/tools/messageTools.ts +++ b/mcp-server/src/tools/messageTools.ts @@ -19,14 +19,39 @@ export function registerMessageTools(server: Pick) { text: z.string().min(1), from: z.string().optional(), summary: z.string().optional(), + source: z.string().optional(), + leadSessionId: z.string().optional(), + attachments: z + .array( + z.object({ + id: z.string().min(1), + filename: z.string().min(1), + mimeType: z.string().min(1), + size: z.number().nonnegative(), + }) + ) + .optional(), }), - execute: async ({ teamName, claudeDir, to, text, from, summary }) => + execute: async ({ + teamName, + claudeDir, + to, + text, + from, + summary, + source, + leadSessionId, + attachments, + }) => jsonTextContent( getController(teamName, claudeDir).messages.sendMessage({ to, text, ...(from ? { from } : {}), ...(summary ? { summary } : {}), + ...(source ? { source } : {}), + ...(leadSessionId ? { leadSessionId } : {}), + ...(attachments?.length ? { attachments } : {}), }) ), }); diff --git a/mcp-server/src/tools/reviewTools.ts b/mcp-server/src/tools/reviewTools.ts index 8faf62e1..a263ad5d 100644 --- a/mcp-server/src/tools/reviewTools.ts +++ b/mcp-server/src/tools/reviewTools.ts @@ -10,6 +10,24 @@ const toolContextSchema = { }; export function registerReviewTools(server: Pick) { + server.addTool({ + name: 'review_request', + description: 'Move a completed task into review and notify reviewer', + parameters: z.object({ + ...toolContextSchema, + taskId: z.string().min(1), + from: z.string().optional(), + reviewer: z.string().optional(), + }), + execute: async ({ teamName, claudeDir, taskId, from, reviewer }) => + jsonTextContent( + getController(teamName, claudeDir).review.requestReview(taskId, { + ...(from ? { from } : {}), + ...(reviewer ? { reviewer } : {}), + }) + ), + }); + server.addTool({ name: 'review_approve', description: 'Approve task review and move kanban state accordingly', diff --git a/mcp-server/test/tools.test.ts b/mcp-server/test/tools.test.ts index 9a1901df..956481f0 100644 --- a/mcp-server/test/tools.test.ts +++ b/mcp-server/test/tools.test.ts @@ -98,6 +98,25 @@ describe('agent-teams-mcp tools', () => { expect(loadedTask.needsClarification).toBe('user'); expect(loadedTask.comments).toHaveLength(1); expect(loadedTask.comments[0].attachments).toHaveLength(1); + + await getTool('task_set_status').execute({ + claudeDir, + teamName, + taskId: createdTask.id, + status: 'completed', + }); + + const reviewRequested = parseJsonToolResult( + await getTool('review_request').execute({ + claudeDir, + teamName, + taskId: createdTask.id, + from: 'lead', + reviewer: 'alice', + }) + ); + + expect(reviewRequested.reviewState).toBe('review'); }); it('covers process register/list/stop without legacy stdout leaking into results', async () => { @@ -141,4 +160,30 @@ describe('agent-teams-mcp tools', () => { expect(stopped.pid).toBe(pid); expect(typeof stopped.stoppedAt).toBe('string'); }); + + it('persists full message metadata through message_send', async () => { + const claudeDir = makeClaudeDir(); + const teamName = 'gamma'; + + const sent = parseJsonToolResult( + await getTool('message_send').execute({ + claudeDir, + teamName, + to: 'alice', + text: 'Check this', + from: 'lead', + summary: 'Metadata test', + source: 'system_notification', + leadSessionId: 'session-42', + attachments: [{ id: 'att-1', filename: 'note.txt', mimeType: 'text/plain', size: 4 }], + }) + ); + + expect(sent.deliveredToInbox).toBe(true); + const inboxPath = path.join(claudeDir, 'teams', teamName, 'inboxes', 'alice.json'); + const rows = JSON.parse(fs.readFileSync(inboxPath, 'utf8')); + expect(rows[0].source).toBe('system_notification'); + expect(rows[0].leadSessionId).toBe('session-42'); + expect(rows[0].attachments[0].filename).toBe('note.txt'); + }); }); diff --git a/src/main/index.ts b/src/main/index.ts index e711c2f5..58d07297 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -430,6 +430,14 @@ function wireFileWatcherEvents(context: ServiceContext): void { // --- Inbox change events: relay to lead + native OS notifications --- if (row.type === 'inbox') { + if (teamDataService) { + void teamDataService + .reconcileTeamArtifacts(teamName) + .catch((e: unknown) => + logger.warn(`[FileWatcher] reconcile failed for ${teamName}: ${String(e)}`) + ); + } + // Auto-relay ONLY lead-inbox changes into the live lead process. // (Relaying on *any* inbox change causes the lead to process irrelevant status noise.) if (teamProvisioningService.isTeamAlive(teamName) && detail.startsWith('inboxes/')) { @@ -479,6 +487,12 @@ function wireFileWatcherEvents(context: ServiceContext): void { // --- Task change events: notify lead when teammate starts a task via CLI --- if (row.type === 'task' && detail.endsWith('.json') && teamDataService) { + void teamDataService + .reconcileTeamArtifacts(teamName) + .catch((e: unknown) => + logger.warn(`[FileWatcher] task reconcile failed for ${teamName}: ${String(e)}`) + ); + const taskId = detail.replace('.json', ''); void teamDataService .notifyLeadOnTeammateTaskStart(teamName, taskId) diff --git a/src/main/services/team/TeamDataService.ts b/src/main/services/team/TeamDataService.ts index eee8b2a6..a6edaf1e 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -1,5 +1,4 @@ import { yieldToEventLoop } from '@main/utils/asyncYield'; -import { readFileUtf8WithTimeout } from '@main/utils/fsRead'; import { encodePath, extractBaseDir, @@ -8,7 +7,6 @@ import { getTasksBasePath, getTeamsBasePath, } from '@main/utils/pathDecoder'; -import { isProcessAlive } from '@main/utils/processHealth'; import { killProcessByPid } from '@main/utils/processKill'; import { AGENT_BLOCK_CLOSE, @@ -17,6 +15,7 @@ import { } from '@shared/constants/agentBlocks'; import { getMemberColor } from '@shared/constants/memberColors'; import { createLogger } from '@shared/utils/logger'; +import { getKanbanColumnFromReviewState, normalizeReviewState } from '@shared/utils/reviewState'; import { formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { parseNumericSuffixName } from '@shared/utils/teamMemberName'; import { extractToolPreview, formatToolSummaryFromCalls } from '@shared/utils/toolSummary'; @@ -46,7 +45,6 @@ import type { InboxMessage, KanbanColumnId, KanbanState, - KanbanTaskState, ResolvedTeamMember, SendMessageRequest, SendMessageResult, @@ -73,7 +71,6 @@ const logger = createLogger('Service:TeamDataService'); const MIN_TEXT_LENGTH = 30; 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; export class TeamDataService { @@ -86,7 +83,7 @@ export class TeamDataService { private readonly configReader: TeamConfigReader = new TeamConfigReader(), private readonly taskReader: TeamTaskReader = new TeamTaskReader(), private readonly inboxReader: TeamInboxReader = new TeamInboxReader(), - private readonly inboxWriter: TeamInboxWriter = new TeamInboxWriter(), + _inboxWriter: TeamInboxWriter = new TeamInboxWriter(), private readonly taskWriter: TeamTaskWriter = new TeamTaskWriter(), private readonly memberResolver: TeamMemberResolver = new TeamMemberResolver(), private readonly kanbanManager: TeamKanbanManager = new TeamKanbanManager(), @@ -108,6 +105,28 @@ export class TeamDataService { return formatTaskDisplayLabel(task); } + private resolveTaskReviewState( + task: Pick, + kanbanState?: Pick + ): 'none' | 'review' | 'approved' { + const explicit = normalizeReviewState(task.reviewState); + if (explicit !== 'none') { + return explicit; + } + + const overlay = kanbanState?.tasks?.[task.id]?.column; + return overlay === 'review' || overlay === 'approved' ? overlay : 'none'; + } + + private attachKanbanCompatibility(task: TeamTask, kanbanState?: KanbanState): TeamTaskWithKanban { + const reviewState = this.resolveTaskReviewState(task, kanbanState); + return { + ...task, + reviewState, + kanbanColumn: getKanbanColumnFromReviewState(reviewState), + }; + } + async listTeams(): Promise { return this.configReader.listTeams(); } @@ -153,11 +172,8 @@ export class TeamDataService { } const info = teamInfoMap.get(task.teamName)!; const kanban = kanbanByTeam.get(task.teamName); - const kanbanEntry = kanban?.tasks[task.id]; - const kanbanColumn = - kanbanEntry?.column === 'review' || kanbanEntry?.column === 'approved' - ? kanbanEntry.column - : undefined; + const reviewState = this.resolveTaskReviewState(task, kanban); + const kanbanColumn = getKanbanColumnFromReviewState(reviewState); // IPC payload safety: GlobalTask lists can be enormous (especially comments and large nested fields). // Return a "light" task object and defer heavy details to team/task detail views. @@ -176,6 +192,7 @@ export class TeamDataService { projectPath, needsClarification: task.needsClarification, deletedAt: task.deletedAt, + reviewState, // Intentionally omit description/comments/activeForm/workIntervals/links to keep payload small kanbanColumn, teamName: task.teamName, @@ -258,12 +275,10 @@ export class TeamDataService { const warnings: string[] = []; let tasks: TeamTask[] = []; - let tasksLoaded = true; try { tasks = await this.taskReader.getTasks(teamName); } catch { warnings.push('Tasks failed to load'); - tasksLoaded = false; } mark('tasks'); @@ -407,21 +422,11 @@ export class TeamDataService { } mark('kanbanState'); - if (canRunKanbanGc && tasksLoaded) { - try { - await this.kanbanManager.garbageCollect(teamName, new Set(tasks.map((task) => task.id))); - kanbanState = await this.kanbanManager.getState(teamName); - } catch { - warnings.push('Kanban state cleanup failed'); - } - } mark('kanbanGc'); - const tasksWithKanban: TeamTaskWithKanban[] = tasks.map((task) => { - const col = kanbanState.tasks[task.id]?.column; - const kanbanColumn = col === 'review' || col === 'approved' ? col : undefined; - return { ...task, kanbanColumn }; - }); + const tasksWithKanban: TeamTaskWithKanban[] = tasks.map((task) => + this.attachKanbanCompatibility(task, canRunKanbanGc ? kanbanState : undefined) + ); const members = this.memberResolver.resolveMembers( config, @@ -436,25 +441,11 @@ export class TeamDataService { await this.enrichMemberBranches(members, config); mark('enrichBranches'); - // Auto-sync: create comments from task-related inbox messages - if (tasksLoaded && messages.length > 0) { - try { - const didSync = await this.syncLinkedComments(teamName, tasks, messages); - if (didSync) { - // Re-read tasks only if new comments were actually written - tasks = await this.taskReader.getTasks(teamName); - } - } catch { - warnings.push('Comment sync from messages failed'); - } - } mark('syncComments'); - const tasksToReturn: TeamTaskWithKanban[] = tasks.map((task) => { - const col = kanbanState.tasks[task.id]?.column; - const kanbanColumn = col === 'review' || col === 'approved' ? col : undefined; - return { ...task, kanbanColumn }; - }); + const tasksToReturn: TeamTaskWithKanban[] = tasks.map((task) => + this.attachKanbanCompatibility(task, canRunKanbanGc ? kanbanState : undefined) + ); let processes: TeamProcess[] = []; try { @@ -1090,7 +1081,15 @@ export class TeamDataService { // non-critical } } - return this.inboxWriter.sendMessage(teamName, enrichedRequest); + return this.getController(teamName).messages.sendMessage({ + member: enrichedRequest.member, + from: enrichedRequest.from, + text: enrichedRequest.text, + summary: enrichedRequest.summary, + source: enrichedRequest.source, + leadSessionId: enrichedRequest.leadSessionId, + attachments: enrichedRequest.attachments, + }) as SendMessageResult; } private resolveLeadNameFromConfig(config: TeamConfig | null): string { @@ -1121,8 +1120,6 @@ export class TeamDataService { summary?: string, attachments?: AttachmentMeta[] ): Promise { - const messageId = randomUUID(); - let leadSessionId: string | undefined; try { const config = await this.configReader.getConfig(teamName); @@ -1131,20 +1128,20 @@ export class TeamDataService { // non-critical — proceed without sessionId } - const msg: InboxMessage = { + const msg = this.getController(teamName).messages.appendSentMessage({ from: 'user', to: leadName, text, - timestamp: new Date().toISOString(), - read: true, summary, - messageId, source: 'user_sent', attachments: attachments?.length ? attachments : undefined, leadSessionId, + }) as InboxMessage; + return { + deliveredToInbox: false, + deliveredViaStdin: true, + messageId: msg.messageId ?? randomUUID(), }; - await this.sentMessagesStore.appendMessage(teamName, msg); - return { deliveredToInbox: false, deliveredViaStdin: true, messageId }; } async getLeadMemberName(teamName: string): Promise { @@ -1175,43 +1172,8 @@ export class TeamDataService { } async requestReview(teamName: string, taskId: string): Promise { - const tasks = await this.taskReader.getTasks(teamName); - const task = tasks.find((candidate) => candidate.id === taskId); - if (!task) { - throw new Error(`Task not found: ${taskId}`); - } - if (task.status !== 'completed') { - throw new Error(`Task ${this.getTaskLabel(task)} must be completed before review`); - } - - this.getController(teamName).kanban.setKanbanColumn(taskId, 'review'); - - const state = await this.kanbanManager.getState(teamName); - const reviewer = state.reviewers[0]; - if (!reviewer) { - return; - } - - try { - const leadName = await this.resolveLeadName(teamName); - await this.sendMessage(teamName, { - member: reviewer, - from: leadName, - text: - `Please review task ${this.getTaskLabel(task)}.\n\n` + - `${AGENT_BLOCK_OPEN}\n` + - `When approved, use MCP tool review_approve:\n` + - `{ teamName: "${teamName}", taskId: "${taskId}", notifyOwner: true }\n\n` + - `If changes are needed, use MCP tool review_request_changes:\n` + - `{ teamName: "${teamName}", taskId: "${taskId}", comment: "..." }\n` + - AGENT_BLOCK_CLOSE, - summary: `Review request for ${this.getTaskLabel(task)}`, - source: 'system_notification', - }); - } catch (error) { - this.getController(teamName).kanban.clearKanban(taskId); - throw error; - } + const leadName = await this.resolveLeadName(teamName); + this.getController(teamName).review.requestReview(taskId, { from: leadName }); } async createTeamConfig(request: TeamCreateConfigRequest): Promise { @@ -1267,6 +1229,18 @@ export class TeamDataService { ); } + async reconcileTeamArtifacts(teamName: string): Promise { + const tasks = await this.taskReader.getTasks(teamName); + await this.kanbanManager.garbageCollect(teamName, new Set(tasks.map((task) => task.id))); + + const messages = await this.inboxReader.getMessages(teamName); + if (messages.length === 0) { + return; + } + + await this.syncLinkedComments(teamName, tasks, messages); + } + /** * Scans inbox messages for task-related discussions and auto-creates * linked comments on disk. Uses deterministic comment ID for dedup. @@ -1329,14 +1303,15 @@ export class TeamDataService { if (existing.some((c) => c.id === commentId)) continue; try { - await this.taskWriter.addComment(teamName, task.id, msg.text, { + this.getController(teamName).tasks.addTaskComment(task.id, { + text: msg.text, id: commentId, - author: msg.from, + from: msg.from, createdAt: msg.timestamp, }); synced = true; } catch { - // Best-effort — don't fail getTeamData() on sync errors + // Best-effort — don't fail reconciliation on sync errors } } } @@ -1511,7 +1486,8 @@ export class TeamDataService { if (patch.op === 'set_column') { if (patch.column === 'review') { - controller.kanban.setKanbanColumn(taskId, 'review'); + const leadName = await this.resolveLeadName(teamName); + controller.review.requestReview(taskId, { from: leadName }); } else { const leadName = await this.resolveLeadName(teamName); controller.review.approveReview(taskId, { diff --git a/src/main/services/team/TeamInboxReader.ts b/src/main/services/team/TeamInboxReader.ts index 0e9a8849..0504e5d9 100644 --- a/src/main/services/team/TeamInboxReader.ts +++ b/src/main/services/team/TeamInboxReader.ts @@ -101,6 +101,21 @@ export class TeamInboxReader { messageId: typeof row.messageId === 'string' ? row.messageId : undefined, source: typeof row.source === 'string' ? (row.source as InboxMessage['source']) : undefined, leadSessionId: typeof row.leadSessionId === 'string' ? row.leadSessionId : undefined, + attachments: Array.isArray(row.attachments) ? row.attachments : undefined, + toolSummary: typeof row.toolSummary === 'string' ? row.toolSummary : undefined, + toolCalls: Array.isArray(row.toolCalls) + ? (row.toolCalls as unknown[]) + .filter( + (tc): tc is { name: string; preview?: string } => + tc != null && + typeof tc === 'object' && + typeof (tc as Record).name === 'string' + ) + .map((tc) => ({ + name: tc.name, + preview: typeof tc.preview === 'string' ? tc.preview : undefined, + })) + : undefined, }); } diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 69c1abaf..46f8e074 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -24,6 +24,7 @@ import { createLogger } from '@shared/utils/logger'; import { formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { createCliAutoSuffixNameGuard } from '@shared/utils/teamMemberName'; import { extractToolPreview, formatToolSummaryFromCalls } from '@shared/utils/toolSummary'; +import * as agentTeamsControllerModule from 'agent-teams-controller'; import { spawn } from 'child_process'; import { randomUUID } from 'crypto'; import * as fs from 'fs'; @@ -58,6 +59,7 @@ import type { } from '@shared/types'; const logger = createLogger('Service:TeamProvisioning'); +const { createController } = agentTeamsControllerModule; const RUN_TIMEOUT_MS = 300_000; const VERIFY_TIMEOUT_MS = 15_000; const VERIFY_POLL_MS = 500; @@ -1083,7 +1085,7 @@ export class TeamProvisioningService { private readonly configReader: TeamConfigReader = new TeamConfigReader(), private readonly inboxReader: TeamInboxReader = new TeamInboxReader(), private readonly membersMetaStore: TeamMembersMetaStore = new TeamMembersMetaStore(), - private readonly sentMessagesStore: TeamSentMessagesStore = new TeamSentMessagesStore(), + _sentMessagesStore: TeamSentMessagesStore = new TeamSentMessagesStore(), private readonly mcpConfigBuilder: TeamMcpConfigBuilder = new TeamMcpConfigBuilder() ) {} @@ -1197,6 +1199,30 @@ export class TeamProvisioningService { this.teamChangeEmitter = emitter; } + private persistSentMessage(teamName: string, message: InboxMessage): void { + try { + createController({ + teamName, + claudeDir: getClaudeBasePath(), + }).messages.appendSentMessage({ + from: message.from, + to: message.to, + text: message.text, + timestamp: message.timestamp, + summary: message.summary, + messageId: message.messageId, + source: message.source, + leadSessionId: message.leadSessionId, + attachments: message.attachments, + color: message.color, + toolSummary: message.toolSummary, + toolCalls: message.toolCalls, + }); + } catch (error) { + logger.warn(`[${teamName}] sent-message persist failed: ${String(error)}`); + } + } + private toolApprovalEventEmitter: ((event: ToolApprovalEvent) => void) | null = null; setToolApprovalEventEmitter(emitter: (event: ToolApprovalEvent) => void): void { @@ -2611,11 +2637,7 @@ export class TeamProvisioningService { }; this.pushLiveLeadProcessMessage(teamName, relayMsg); // Persist to disk so relayed replies survive app restart and trigger FileWatcher - void this.sentMessagesStore - .appendMessage(teamName, relayMsg) - .catch((e: unknown) => - logger.warn(`[${teamName}] sentMessagesStore persist failed: ${String(e)}`) - ); + this.persistSentMessage(teamName, relayMsg); this.teamChangeEmitter?.({ type: 'inbox', teamName, @@ -2831,13 +2853,7 @@ export class TeamProvisioningService { }; this.pushLiveLeadProcessMessage(run.teamName, msg); - void this.sentMessagesStore - .appendMessage(run.teamName, msg) - .catch((e: unknown) => - logger.warn( - `[${run.teamName}] sentMessagesStore persist (SendMessage capture) failed: ${String(e)}` - ) - ); + this.persistSentMessage(run.teamName, msg); this.teamChangeEmitter?.({ type: 'inbox', teamName: run.teamName, diff --git a/src/main/services/team/TeamTaskReader.ts b/src/main/services/team/TeamTaskReader.ts index e888b80d..d1833658 100644 --- a/src/main/services/team/TeamTaskReader.ts +++ b/src/main/services/team/TeamTaskReader.ts @@ -2,6 +2,7 @@ import { yieldToEventLoop } from '@main/utils/asyncYield'; import { readFileUtf8WithTimeout } from '@main/utils/fsRead'; import { getTasksBasePath } from '@main/utils/pathDecoder'; import { createLogger } from '@shared/utils/logger'; +import { normalizeReviewState } from '@shared/utils/reviewState'; import { deriveTaskDisplayId } from '@shared/utils/taskIdentity'; import * as fs from 'fs'; import * as path from 'path'; @@ -275,6 +276,7 @@ export class TeamTaskReader { addedAt: a.addedAt, })) : undefined, + reviewState: normalizeReviewState(parsed.reviewState), } satisfies Record; if (task.status === 'deleted') { continue; @@ -376,6 +378,7 @@ export class TeamTaskReader { status: 'deleted', deletedAt: typeof parsed.deletedAt === 'string' ? parsed.deletedAt : undefined, createdAt: typeof parsed.createdAt === 'string' ? parsed.createdAt : undefined, + reviewState: normalizeReviewState(parsed.reviewState), }; tasks.push(task); diff --git a/src/main/workers/team-fs-worker.ts b/src/main/workers/team-fs-worker.ts index fc86fbf0..2e43f193 100644 --- a/src/main/workers/team-fs-worker.ts +++ b/src/main/workers/team-fs-worker.ts @@ -117,6 +117,7 @@ interface ParsedTask { projectPath?: unknown; comments?: unknown; needsClarification?: unknown; + reviewState?: unknown; metadata?: { _internal?: unknown }; workIntervals?: unknown; statusHistory?: unknown; @@ -600,6 +601,12 @@ async function readTasksDirForTeam( parsed.needsClarification === 'lead' || parsed.needsClarification === 'user' ? (parsed.needsClarification as string) : undefined; + const reviewState = + parsed.reviewState === 'review' || + parsed.reviewState === 'approved' || + parsed.reviewState === 'none' + ? parsed.reviewState + : 'none'; tasks.push({ id: typeof parsed.id === 'string' || typeof parsed.id === 'number' ? String(parsed.id) : '', @@ -635,6 +642,7 @@ async function readTasksDirForTeam( projectPath: typeof parsed.projectPath === 'string' ? parsed.projectPath : undefined, comments: normalizeComments(parsed), needsClarification, + reviewState, deletedAt: undefined, attachments: Array.isArray(parsed.attachments) ? (parsed.attachments as unknown[]) diff --git a/src/renderer/components/settings/sections/NotificationsSection.tsx b/src/renderer/components/settings/sections/NotificationsSection.tsx index 92aeb124..0ca33a92 100644 --- a/src/renderer/components/settings/sections/NotificationsSection.tsx +++ b/src/renderer/components/settings/sections/NotificationsSection.tsx @@ -14,10 +14,10 @@ import { NotificationTriggerSettings } from '../NotificationTriggerSettings'; import type { RepositoryDropdownItem, SafeConfig } from '../hooks/useSettingsConfig'; import type { NotificationTrigger } from '@renderer/types/data'; -import type { TeamTaskStatus } from '@shared/types'; +import type { TeamReviewState, TeamTaskStatus } from '@shared/types'; -/** Statuses available for notification filtering — real status values + kanban-only 'approved'. */ -type NotifiableStatus = TeamTaskStatus | 'approved'; +/** Notification targets span workflow status plus the explicit review axis. */ +type NotifiableStatus = TeamTaskStatus | Extract; // Snooze duration options const SNOOZE_OPTIONS = [ diff --git a/src/renderer/components/sidebar/SidebarTaskItem.tsx b/src/renderer/components/sidebar/SidebarTaskItem.tsx index e1aad9c8..15c7a711 100644 --- a/src/renderer/components/sidebar/SidebarTaskItem.tsx +++ b/src/renderer/components/sidebar/SidebarTaskItem.tsx @@ -9,6 +9,7 @@ import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { nameColorSet } from '@renderer/utils/projectColor'; import { projectColor } from '@renderer/utils/projectColor'; import { projectLabelFromPath } from '@renderer/utils/taskGrouping'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { format, isThisYear, isToday, isYesterday } from 'date-fns'; import { CheckCircle2, Circle, Eye, Loader2, ShieldCheck, Trash2 } from 'lucide-react'; @@ -103,10 +104,11 @@ export const SidebarTaskItem = ({ } }, [isRenaming, displaySubject]); + const reviewColumn = getTaskKanbanColumn(task); const cfg = - task.kanbanColumn === 'approved' + reviewColumn === 'approved' ? ({ icon: ShieldCheck, color: 'text-teal-400', label: 'approved' } as const) - : task.kanbanColumn === 'review' + : reviewColumn === 'review' ? ({ icon: Eye, color: 'text-orange-400', label: 'in review' } as const) : (statusConfig[task.status] ?? statusConfig.pending); const StatusIcon = cfg.icon; diff --git a/src/renderer/components/sidebar/taskFiltersState.ts b/src/renderer/components/sidebar/taskFiltersState.ts index 34a1a466..5d6ffe13 100644 --- a/src/renderer/components/sidebar/taskFiltersState.ts +++ b/src/renderer/components/sidebar/taskFiltersState.ts @@ -1,6 +1,7 @@ import { useSyncExternalStore } from 'react'; import { getSnapshot, getUnreadCount, subscribe } from '@renderer/services/commentReadStorage'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; export type TaskStatusFilterId = 'todo' | 'in_progress' | 'done' | 'review' | 'approved'; @@ -25,17 +26,22 @@ export const defaultTaskFiltersState = (): TaskFiltersState => ({ }); export function taskMatchesStatus( - task: { status: string; kanbanColumn?: 'review' | 'approved' }, + task: { + status: string; + reviewState?: 'none' | 'review' | 'approved'; + kanbanColumn?: 'review' | 'approved'; + }, statusIds: Set ): boolean { if (statusIds.size === 0) return false; if (statusIds.size === STATUS_OPTIONS.length) return task.status !== 'deleted'; - const inTodo = task.status === 'pending' && !task.kanbanColumn; - const inProgress = task.status === 'in_progress' && !task.kanbanColumn; - const inDone = task.status === 'completed' && !task.kanbanColumn; - const inReview = task.kanbanColumn === 'review'; - const inApproved = task.kanbanColumn === 'approved'; + const kanbanColumn = getTaskKanbanColumn(task); + const inTodo = task.status === 'pending' && !kanbanColumn; + const inProgress = task.status === 'in_progress' && !kanbanColumn; + const inDone = task.status === 'completed' && !kanbanColumn; + const inReview = kanbanColumn === 'review'; + const inApproved = kanbanColumn === 'approved'; return ( (statusIds.has('todo') && inTodo) || diff --git a/src/renderer/components/team/TaskTooltip.tsx b/src/renderer/components/team/TaskTooltip.tsx index 27d82840..cba866f1 100644 --- a/src/renderer/components/team/TaskTooltip.tsx +++ b/src/renderer/components/team/TaskTooltip.tsx @@ -5,6 +5,7 @@ import { MemberBadge } from '@renderer/components/team/MemberBadge'; import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; import { useStore } from '@renderer/store'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { formatTaskDisplayLabel, taskMatchesRef } from '@shared/utils/taskIdentity'; import type { TeamTaskWithKanban } from '@shared/types'; @@ -25,7 +26,8 @@ const STATUS_COLORS: Record = { }; function getEffectiveColumn(task: TeamTaskWithKanban): string { - if (task.kanbanColumn) return task.kanbanColumn; + const reviewColumn = getTaskKanbanColumn(task); + if (reviewColumn) return reviewColumn; if (task.status === 'pending') return 'todo'; if (task.status === 'completed') return 'done'; return task.status; diff --git a/src/renderer/components/team/dialogs/CreateTaskDialog.tsx b/src/renderer/components/team/dialogs/CreateTaskDialog.tsx index 7acde5c9..02f3b59d 100644 --- a/src/renderer/components/team/dialogs/CreateTaskDialog.tsx +++ b/src/renderer/components/team/dialogs/CreateTaskDialog.tsx @@ -28,8 +28,9 @@ import { useStore } from '@renderer/store'; import { chipToken, serializeChipsWithText } from '@renderer/types/inlineChip'; import { removeChipTokenFromText } from '@renderer/utils/chipUtils'; import { formatAgentRole } from '@renderer/utils/formatAgentRole'; -import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; +import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { AlertTriangle, Search } from 'lucide-react'; import type { InlineChip } from '@renderer/types/inlineChip'; @@ -146,7 +147,7 @@ export const CreateTaskDialog = ({ // Only show non-internal, non-deleted tasks as candidates for blocking const availableTasks = tasks.filter( - (t) => t.status !== 'deleted' && t.kanbanColumn !== 'approved' + (t) => t.status !== 'deleted' && getTaskKanbanColumn(t) !== 'approved' ); const toggleBlockedBy = (taskId: string): void => { diff --git a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx index c8406e73..9c30dc4e 100644 --- a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx +++ b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx @@ -22,7 +22,6 @@ import { Textarea } from '@renderer/components/ui/textarea'; import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; import { markAsRead } from '@renderer/services/commentReadStorage'; import { useStore } from '@renderer/store'; -import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { isImageMimeType } from '@renderer/utils/attachmentUtils'; import { buildMemberColorMap, @@ -30,6 +29,8 @@ import { TASK_STATUS_LABELS, TASK_STATUS_STYLES, } from '@renderer/utils/memberHelpers'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; +import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { formatDistanceToNow } from 'date-fns'; import { AlignLeft, @@ -287,7 +288,12 @@ export const TaskDetailDialog = ({ ); } - const kanbanColumn = kanbanTaskState?.column ?? currentTask.kanbanColumn; + const kanbanColumn = + kanbanTaskState?.column ?? + getTaskKanbanColumn({ + reviewState: currentTask.reviewState, + kanbanColumn: currentTask.kanbanColumn, + }); const status = currentTask.status; const statusStyle = kanbanColumn && KANBAN_COLUMN_DISPLAY[kanbanColumn] diff --git a/src/renderer/components/team/kanban/KanbanBoard.tsx b/src/renderer/components/team/kanban/KanbanBoard.tsx index 3caacd0f..0271eb70 100644 --- a/src/renderer/components/team/kanban/KanbanBoard.tsx +++ b/src/renderer/components/team/kanban/KanbanBoard.tsx @@ -8,6 +8,7 @@ import { Button } from '@renderer/components/ui/button'; import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; import { useResizableColumns } from '@renderer/hooks/useResizableColumns'; import { cn } from '@renderer/lib/utils'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { CheckCircle2, ClipboardList, @@ -105,9 +106,12 @@ const COLUMNS: { id: KanbanColumnId; title: string }[] = [ ]; function getTaskColumn(task: TeamTask, kanbanState: KanbanState): KanbanColumnId | null { - const explicit = kanbanState.tasks[task.id]; - if (explicit?.column) { - return explicit.column; + const explicit = getTaskKanbanColumn({ + reviewState: task.reviewState, + kanbanColumn: kanbanState.tasks[task.id]?.column, + }); + if (explicit) { + return explicit; } if (task.status === 'pending') { diff --git a/src/renderer/components/team/members/MemberTasksTab.tsx b/src/renderer/components/team/members/MemberTasksTab.tsx index f71676bc..7c67e028 100644 --- a/src/renderer/components/team/members/MemberTasksTab.tsx +++ b/src/renderer/components/team/members/MemberTasksTab.tsx @@ -6,6 +6,7 @@ import { TASK_STATUS_LABELS, TASK_STATUS_STYLES, } from '@renderer/utils/memberHelpers'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import type { TeamTaskWithKanban } from '@shared/types'; @@ -42,7 +43,7 @@ export const MemberTasksTab = ({ tasks, onTaskClick }: MemberTasksTabProps): Rea
{visibleTasks.map((task) => { - const col = task.kanbanColumn; + const col = getTaskKanbanColumn(task); const style = col && KANBAN_COLUMN_DISPLAY[col] ? { bg: KANBAN_COLUMN_DISPLAY[col].bg, text: KANBAN_COLUMN_DISPLAY[col].text } diff --git a/src/renderer/components/team/tasks/TaskRow.tsx b/src/renderer/components/team/tasks/TaskRow.tsx index 97bd11cb..6a74826c 100644 --- a/src/renderer/components/team/tasks/TaskRow.tsx +++ b/src/renderer/components/team/tasks/TaskRow.tsx @@ -1,4 +1,5 @@ import { KANBAN_COLUMN_DISPLAY, TASK_STATUS_LABELS } from '@renderer/utils/memberHelpers'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import type { TeamTaskWithKanban } from '@shared/types'; @@ -10,6 +11,7 @@ interface TaskRowProps { export const TaskRow = ({ task }: TaskRowProps): React.JSX.Element => { const blockedByIds = task.blockedBy?.filter((id) => id.length > 0) ?? []; const blocksIds = task.blocks?.filter((id) => id.length > 0) ?? []; + const kanbanColumn = getTaskKanbanColumn(task); return ( @@ -21,8 +23,8 @@ export const TaskRow = ({ task }: TaskRowProps): React.JSX.Element => { {task.owner ?? 'Unassigned'} - {task.kanbanColumn && task.kanbanColumn in KANBAN_COLUMN_DISPLAY - ? KANBAN_COLUMN_DISPLAY[task.kanbanColumn].label + {kanbanColumn && kanbanColumn in KANBAN_COLUMN_DISPLAY + ? KANBAN_COLUMN_DISPLAY[kanbanColumn].label : (TASK_STATUS_LABELS[task.status] ?? task.status)} diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index 00d18fbf..2193e1fd 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -2,6 +2,7 @@ import { api } from '@renderer/api'; import { normalizePath } from '@renderer/utils/pathNormalize'; import { IpcError, unwrapIpc } from '@renderer/utils/unwrapIpc'; import { createLogger } from '@shared/utils/logger'; +import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { getWorktreeNavigationState } from '../utils/stateResetHelpers'; @@ -155,7 +156,9 @@ function detectStatusChangeNotifications( if (!oldTask) continue; // Detect kanbanColumn change to 'approved' (status stays 'completed', column changes) - const becameApproved = task.kanbanColumn === 'approved' && oldTask.kanbanColumn !== 'approved'; + const taskKanbanColumn = getTaskKanbanColumn(task); + const oldTaskKanbanColumn = getTaskKanbanColumn(oldTask); + const becameApproved = taskKanbanColumn === 'approved' && oldTaskKanbanColumn !== 'approved'; const statusChanged = oldTask.status !== task.status; if (!statusChanged && !becameApproved) continue; @@ -511,7 +514,7 @@ export const createTeamSlice: StateCreator = (set, notifiedClarificationTaskKeys.add(`${task.teamName}:${task.id}`); } notifiedStatusChangeKeys.add(`${task.teamName}:${task.id}:${task.status}`); - if (task.kanbanColumn === 'approved') { + if (getTaskKanbanColumn(task) === 'approved') { notifiedStatusChangeKeys.add(`${task.teamName}:${task.id}:approved`); } } diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index 78eb8539..359767cd 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -56,6 +56,7 @@ export interface TeamSummary { } export type TeamTaskStatus = 'pending' | 'in_progress' | 'completed' | 'deleted'; +export type TeamReviewState = 'none' | 'review' | 'approved'; export interface TaskWorkInterval { /** ISO timestamp when task entered in_progress */ @@ -129,6 +130,8 @@ export interface TeamTask { deletedAt?: string; /** Attachments associated with this task. Metadata only — actual files stored on disk. */ attachments?: TaskAttachmentMeta[]; + /** Separate review lifecycle axis. Persisted on modern tasks, derived for legacy rows when needed. */ + reviewState?: TeamReviewState; } /** Task enriched for UI/DTO use (overlay from kanban-state.json). */ diff --git a/src/shared/utils/reviewState.ts b/src/shared/utils/reviewState.ts new file mode 100644 index 00000000..4706cedf --- /dev/null +++ b/src/shared/utils/reviewState.ts @@ -0,0 +1,42 @@ +import type { TeamReviewState } from '@shared/types'; + +interface ReviewStateLike { + reviewState?: TeamReviewState | null; + kanbanColumn?: 'review' | 'approved' | null; + status?: string | null; +} + +export function normalizeReviewState(value: unknown): TeamReviewState { + return value === 'review' || value === 'approved' ? value : 'none'; +} + +export function getReviewStateFromTask(task: ReviewStateLike): TeamReviewState { + const explicit = normalizeReviewState(task.reviewState); + if (explicit !== 'none') { + return explicit; + } + + if (task.kanbanColumn === 'review' || task.kanbanColumn === 'approved') { + return task.kanbanColumn; + } + + return 'none'; +} + +export function getKanbanColumnFromReviewState( + reviewState: TeamReviewState +): 'review' | 'approved' | undefined { + return reviewState === 'review' || reviewState === 'approved' ? reviewState : undefined; +} + +export function getTaskKanbanColumn(task: ReviewStateLike): 'review' | 'approved' | undefined { + return getKanbanColumnFromReviewState(getReviewStateFromTask(task)); +} + +export function isApprovedTask(task: ReviewStateLike): boolean { + return getReviewStateFromTask(task) === 'approved'; +} + +export function isReviewTask(task: ReviewStateLike): boolean { + return getReviewStateFromTask(task) === 'review'; +} diff --git a/src/types/agent-teams-controller.d.ts b/src/types/agent-teams-controller.d.ts index 71c99de1..07f85bb6 100644 --- a/src/types/agent-teams-controller.d.ts +++ b/src/types/agent-teams-controller.d.ts @@ -39,11 +39,13 @@ declare module 'agent-teams-controller' { } export interface ControllerReviewApi { + requestReview(taskId: string, flags?: Record): unknown; approveReview(taskId: string, flags?: Record): unknown; requestChanges(taskId: string, flags?: Record): unknown; } export interface ControllerMessageApi { + appendSentMessage(flags: Record): unknown; sendMessage(flags: Record): unknown; } diff --git a/test/main/services/team/TeamDataService.test.ts b/test/main/services/team/TeamDataService.test.ts index 77bf569d..2db057c4 100644 --- a/test/main/services/team/TeamDataService.test.ts +++ b/test/main/services/team/TeamDataService.test.ts @@ -5,7 +5,7 @@ import { TeamDataService } from '../../../../src/main/services/team/TeamDataServ import type { InboxMessage, TeamTask } from '../../../../src/shared/types/team'; describe('TeamDataService', () => { - it('runs kanban garbage-collect only after tasks are loaded', async () => { + it('keeps getTeamData read-only and skips kanban garbage-collect', async () => { const order: string[] = []; const tasks: TeamTask[] = [ { @@ -44,10 +44,10 @@ describe('TeamDataService', () => { ); await service.getTeamData('my-team'); - expect(order).toEqual(['tasks', 'gc']); + expect(order).toEqual(['tasks']); }); - it('does not sync automated comment notifications into task comments', async () => { + it('reconciles linked comments outside getTeamData and skips automated notifications', async () => { const tasks: TeamTask[] = [ { id: '12', @@ -106,14 +106,20 @@ describe('TeamDataService', () => { } as never, { readMessages: vi.fn(async () => []), - } as never + } as never, + () => + ({ + tasks: { + addTaskComment: addComment, + }, + }) as never ); - await service.getTeamData('my-team'); + await service.reconcileTeamArtifacts('my-team'); expect(addComment).not.toHaveBeenCalled(); }); - it('skips kanban garbage-collect when tasks fail to load', async () => { + it('skips reconcile writes when tasks fail to load', async () => { const garbageCollect = vi.fn(async () => undefined); const service = new TeamDataService( { @@ -140,9 +146,8 @@ describe('TeamDataService', () => { } as never ); - const result = await service.getTeamData('my-team'); + await expect(service.reconcileTeamArtifacts('my-team')).rejects.toThrow('tasks failed'); expect(garbageCollect).not.toHaveBeenCalled(); - expect(result.warnings).toContain('Tasks failed to load'); }); it('includes projectPath from config when creating a task', async () => { @@ -292,4 +297,79 @@ describe('TeamDataService', () => { expect(result.related).toEqual(['1', '2']); expect(createTaskMock).toHaveBeenCalledWith(expect.objectContaining({ related: ['1', '2'] })); }); + + it('routes durable inbox writes through controller message API', async () => { + const sendMessageMock = vi.fn(() => ({ deliveredToInbox: true, messageId: 'm-1' })); + + const service = new TeamDataService( + { + listTeams: vi.fn(), + getConfig: vi.fn(async () => ({ name: 'My team', members: [], leadSessionId: 'lead-1' })), + } as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + () => + ({ + messages: { + sendMessage: sendMessageMock, + }, + }) as never + ); + + const result = await service.sendMessage('my-team', { + member: 'alice', + text: 'hello', + summary: 'ping', + }); + + expect(result).toEqual({ deliveredToInbox: true, messageId: 'm-1' }); + expect(sendMessageMock).toHaveBeenCalledWith( + expect.objectContaining({ + member: 'alice', + text: 'hello', + summary: 'ping', + leadSessionId: 'lead-1', + }) + ); + }); + + it('delegates review entry to controller review API', async () => { + const requestReviewMock = vi.fn(); + + const service = new TeamDataService( + { + listTeams: vi.fn(), + getConfig: vi.fn(async () => ({ + name: 'My team', + members: [{ name: 'lead', role: 'team lead' }], + })), + } as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + () => + ({ + review: { + requestReview: requestReviewMock, + }, + }) as never + ); + + await service.requestReview('my-team', 'task-1'); + + expect(requestReviewMock).toHaveBeenCalledWith('task-1', { from: 'lead' }); + }); }); diff --git a/test/main/services/team/TeamProvisioningServiceRelay.test.ts b/test/main/services/team/TeamProvisioningServiceRelay.test.ts index d316f4bb..341a1727 100644 --- a/test/main/services/team/TeamProvisioningServiceRelay.test.ts +++ b/test/main/services/team/TeamProvisioningServiceRelay.test.ts @@ -42,6 +42,14 @@ const hoisted = vi.hoisted(() => { stat, readFile, atomicWrite, + appendSentMessage: vi.fn((teamName: string, message: Record) => { + const sentMessagesPath = `/mock/teams/${teamName}/sentMessages.json`; + const current = files.get(sentMessagesPath); + const rows = current ? (JSON.parse(current) as unknown[]) : []; + rows.push(message); + files.set(sentMessagesPath, JSON.stringify(rows)); + return message; + }), setAtomicWriteShouldFail: (next: boolean) => { atomicWriteShouldFail = next; }, @@ -72,6 +80,15 @@ vi.mock('../../../../src/main/utils/pathDecoder', async (importOriginal) => { }; }); +vi.mock('agent-teams-controller', () => ({ + createController: ({ teamName }: { teamName: string }) => ({ + messages: { + appendSentMessage: (message: Record) => + hoisted.appendSentMessage(teamName, message), + }, + }), +})); + import { TeamProvisioningService } from '../../../../src/main/services/team/TeamProvisioningService'; function seedConfig(teamName: string): void { @@ -139,6 +156,7 @@ describe('TeamProvisioningService relayLeadInboxMessages', () => { hoisted.files.clear(); hoisted.readFile.mockClear(); hoisted.atomicWrite.mockClear(); + hoisted.appendSentMessage.mockClear(); hoisted.setAtomicWriteShouldFail(false); }); @@ -210,14 +228,7 @@ describe('TeamProvisioningService relayLeadInboxMessages', () => { expect(first).toBe(1); expect(second).toBe(0); expect(writeSpy).toHaveBeenCalledTimes(1); - - // Relay now also persists to sentMessages.json via appendMessage() which uses - // atomicWriteAsync — expected to fail here since atomicWriteShouldFail=true. - expect(console.error).toHaveBeenCalledWith( - expect.stringContaining('TeamSentMessagesStore'), - expect.stringContaining('Failed to append sent message') - ); - vi.mocked(console.error).mockClear(); + expect(hoisted.appendSentMessage).toHaveBeenCalledTimes(1); }); it('does not mark as relayed when stdin is not writable', async () => {