From 9adffb2295af258937bd3b4bee1e17d1c0c7d94c Mon Sep 17 00:00:00 2001 From: iliya Date: Mon, 9 Mar 2026 15:25:22 +0200 Subject: [PATCH] feat: enhance task comment notifications and owner wake-up logic - Introduced a new function to notify task owners on comments from other members, improving communication and responsiveness. - Updated existing comment handling functions to include logic for notifying owners based on comment type and author. - Added tests to validate the new notification behavior, ensuring that owners are correctly alerted for relevant comments while avoiding unnecessary notifications for self-comments. - Refactored task management logic to streamline comment processing and notification handling. --- agent-teams-controller/src/internal/review.js | 2 + .../src/internal/taskStore.js | 18 +- agent-teams-controller/src/internal/tasks.js | 65 ++++++ .../test/controller.test.js | 71 ++++++ mcp-server/test/tools.test.ts | 5 + src/main/index.ts | 11 +- src/main/ipc/teams.ts | 7 +- src/main/services/team/TeamDataService.ts | 52 ----- .../services/team/TeamProvisioningService.ts | 213 ++++++++++++++---- .../components/team/UnreadCommentsBadge.tsx | 6 +- .../team/activity/ActivityTimeline.tsx | 2 +- .../team/activity/LeadThoughtsGroup.tsx | 6 +- .../components/team/kanban/KanbanTaskCard.tsx | 4 +- .../team/messages/MessageComposer.tsx | 7 +- .../team/review/CodeMirrorDiffView.tsx | 6 +- .../team/review/ReviewDiffContent.tsx | 2 +- src/renderer/index.css | 8 +- test/main/ipc/teams.test.ts | 1 + ...eamProvisioningServiceLiveMessages.test.ts | 30 +++ .../TeamProvisioningServicePrompts.test.ts | 4 + .../team/TeamProvisioningServiceRelay.test.ts | 30 +++ .../team/activity/LeadThoughtsGroup.test.ts | 18 ++ 22 files changed, 449 insertions(+), 119 deletions(-) create mode 100644 test/renderer/components/team/activity/LeadThoughtsGroup.test.ts diff --git a/agent-teams-controller/src/internal/review.js b/agent-teams-controller/src/internal/review.js index ec0e8040..a665f183 100644 --- a/agent-teams-controller/src/internal/review.js +++ b/agent-teams-controller/src/internal/review.js @@ -131,6 +131,7 @@ function approveReview(context, taskId, flags = {}) { text: note, from, type: 'review_approved', + notifyOwner: false, }); if ((flags.notify === true || flags['notify-owner'] === true) && task.owner) { @@ -184,6 +185,7 @@ function requestChanges(context, taskId, flags = {}) { text: comment, from, type: 'review_request', + notifyOwner: false, }); messages.sendMessage(context, { to: task.owner, diff --git a/agent-teams-controller/src/internal/taskStore.js b/agent-teams-controller/src/internal/taskStore.js index 3d45b1af..f02216e9 100644 --- a/agent-teams-controller/src/internal/taskStore.js +++ b/agent-teams-controller/src/internal/taskStore.js @@ -413,6 +413,10 @@ function updateTaskFields(paths, taskRef, fields) { }); } +function normalizeMemberName(value) { + return typeof value === 'string' && value.trim() ? value.trim().toLowerCase() : ''; +} + function addTaskComment(paths, taskRef, text, options = {}) { if (typeof text !== 'string' || !text.trim()) { throw new Error('Missing comment text'); @@ -435,21 +439,31 @@ function addTaskComment(paths, taskRef, text, options = {}) { : {}), }; + let inserted = false; + let clarificationCleared = false; const task = updateTask(paths, taskRef, (currentTask) => { const comments = Array.isArray(currentTask.comments) ? currentTask.comments : []; if (comments.some((entry) => entry.id === comment.id)) { return currentTask; } - if (currentTask.needsClarification === 'lead' && comment.author !== currentTask.owner) { + const authorName = normalizeMemberName(comment.author); + const ownerName = normalizeMemberName(currentTask.owner); + if (currentTask.needsClarification === 'lead' && authorName && authorName !== ownerName) { delete currentTask.needsClarification; + clarificationCleared = true; + } + if (currentTask.needsClarification === 'user' && authorName === 'user') { + delete currentTask.needsClarification; + clarificationCleared = true; } currentTask.comments = comments.concat([comment]); + inserted = true; return currentTask; }); - return { comment, task }; + return { comment, task, inserted, clarificationCleared }; } function setNeedsClarification(paths, taskRef, value) { diff --git a/agent-teams-controller/src/internal/tasks.js b/agent-teams-controller/src/internal/tasks.js index aed667fe..f18ed959 100644 --- a/agent-teams-controller/src/internal/tasks.js +++ b/agent-teams-controller/src/internal/tasks.js @@ -11,6 +11,22 @@ function isSameMember(left, right) { return normalizeActorName(left).toLowerCase() === normalizeActorName(right).toLowerCase(); } +function isSameTaskMember(left, right, leadName) { + const normalizedLeft = normalizeActorName(left).toLowerCase(); + const normalizedRight = normalizeActorName(right).toLowerCase(); + const normalizedLead = normalizeActorName(leadName).toLowerCase(); + if (!normalizedLeft || !normalizedRight) { + return false; + } + if (normalizedLeft === normalizedRight) { + return true; + } + return ( + (normalizedLeft === 'team-lead' && normalizedRight === normalizedLead) || + (normalizedRight === 'team-lead' && normalizedLeft === normalizedLead) + ); +} + function buildAssignmentMessage(context, task, options = {}) { const description = typeof options.description === 'string' && options.description.trim() @@ -45,6 +61,18 @@ function buildAssignmentMessage(context, task, options = {}) { return lines.join('\n'); } +function buildCommentNotificationMessage(context, task, comment) { + const taskLabel = `#${task.displayId || task.id}`; + return [ + `Comment on task ${taskLabel} "${task.subject}":`, + ``, + comment.text, + ``, + wrapAgentBlock(`Reply to this comment using MCP tool task_add_comment: +{ teamName: "${context.teamName}", taskId: "${task.id}", text: "", from: "" }`), + ].join('\n'); +} + function maybeNotifyAssignedOwner(context, task, options = {}) { const owner = normalizeActorName(task.owner); if (!owner || task.status === 'deleted') { @@ -69,6 +97,38 @@ function maybeNotifyAssignedOwner(context, task, options = {}) { }); } +function maybeNotifyTaskOwnerOnComment(context, task, comment, options = {}) { + if (!options.inserted || options.notifyOwner === false) { + return; + } + if (!task || task.status === 'deleted') { + return; + } + if (comment.type && comment.type !== 'regular') { + return; + } + + const owner = normalizeActorName(task.owner); + if (!owner) { + return; + } + + const leadName = runtimeHelpers.inferLeadName(context.paths); + if (isSameTaskMember(owner, comment.author, leadName)) { + return; + } + + const leadSessionId = runtimeHelpers.resolveLeadSessionId(context.paths); + messages.sendMessage(context, { + member: owner, + from: normalizeActorName(comment.author) || leadName, + text: buildCommentNotificationMessage(context, task, comment), + summary: `Comment on #${task.displayId || task.id}`, + source: 'system_notification', + ...(leadSessionId ? { leadSessionId } : {}), + }); +} + function createTask(context, input) { const task = taskStore.createTask(context.paths, input); if (input && input.notifyOwner !== false) { @@ -152,6 +212,11 @@ function addTaskComment(context, taskId, flags) { ...(Array.isArray(flags.attachments) ? { attachments: flags.attachments } : {}), }); + maybeNotifyTaskOwnerOnComment(context, result.task, result.comment, { + inserted: result.inserted, + notifyOwner: flags.notifyOwner, + }); + return { commentId: result.comment.id, taskId: result.task.id, diff --git a/agent-teams-controller/test/controller.test.js b/agent-teams-controller/test/controller.test.js index c687835f..82d6adb7 100644 --- a/agent-teams-controller/test/controller.test.js +++ b/agent-teams-controller/test/controller.test.js @@ -382,6 +382,77 @@ describe('agent-teams-controller API', () => { expect(rows[0].attachments[0].filename).toBe('note.txt'); }); + it('wakes task owner on regular comment from another member', () => { + const claudeDir = makeClaudeDir(); + const controller = createController({ teamName: 'my-team', claudeDir }); + const task = controller.tasks.createTask({ subject: 'Investigate', owner: 'bob', notifyOwner: false }); + + const commented = controller.tasks.addTaskComment(task.id, { + from: 'alice', + text: 'I found the root cause.', + }); + + expect(commented.task.comments.at(-1).text).toBe('I found the root cause.'); + const inboxPath = path.join(claudeDir, 'teams', 'my-team', 'inboxes', 'bob.json'); + const rows = JSON.parse(fs.readFileSync(inboxPath, 'utf8')); + expect(rows).toHaveLength(1); + expect(rows[0].summary).toContain(`#${task.displayId}`); + expect(rows[0].text).toContain('I found the root cause.'); + expect(rows[0].leadSessionId).toBe('lead-session-1'); + }); + + it('does not wake owner for self-comments and clears user clarification when user replies', () => { + const claudeDir = makeClaudeDir(); + const controller = createController({ teamName: 'my-team', claudeDir }); + const task = controller.tasks.createTask({ + subject: 'Need product input', + owner: 'bob', + needsClarification: 'user', + notifyOwner: false, + }); + + controller.tasks.addTaskComment(task.id, { + from: 'bob', + text: 'Starting to investigate.', + }); + + const ownerInboxPath = path.join(claudeDir, 'teams', 'my-team', 'inboxes', 'bob.json'); + expect(fs.existsSync(ownerInboxPath)).toBe(false); + + const replied = controller.tasks.addTaskComment(task.id, { + from: 'user', + text: 'Please use the safer option.', + }); + + expect(replied.task.needsClarification).toBeUndefined(); + const reloaded = controller.tasks.getTask(task.id); + expect(reloaded.needsClarification).toBeUndefined(); + const rows = JSON.parse(fs.readFileSync(ownerInboxPath, 'utf8')); + expect(rows).toHaveLength(1); + expect(rows[0].text).toContain('Please use the safer option.'); + }); + + it('wakes lead owner on comment from another member', () => { + const claudeDir = makeClaudeDir(); + const controller = createController({ teamName: 'my-team', claudeDir }); + const task = controller.tasks.createTask({ + subject: 'Lead-owned task', + owner: 'team-lead', + notifyOwner: false, + }); + + controller.tasks.addTaskComment(task.id, { + from: 'alice', + text: 'Need your decision here.', + }); + + const inboxPath = path.join(claudeDir, 'teams', 'my-team', 'inboxes', 'team-lead.json'); + const rows = JSON.parse(fs.readFileSync(inboxPath, 'utf8')); + expect(rows).toHaveLength(1); + expect(rows[0].from).toBe('alice'); + expect(rows[0].text).toContain('Need your decision here.'); + }); + it('moves review back to pending+needsFix and notifies owner on requestChanges', () => { const claudeDir = makeClaudeDir(); const controller = createController({ teamName: 'my-team', claudeDir }); diff --git a/mcp-server/test/tools.test.ts b/mcp-server/test/tools.test.ts index 10d19dcd..82bf5cae 100644 --- a/mcp-server/test/tools.test.ts +++ b/mcp-server/test/tools.test.ts @@ -151,6 +151,11 @@ describe('agent-teams-mcp tools', () => { const commentId = commented.commentId; expect(commentId).toBeTruthy(); + const ownerInboxPath = path.join(claudeDir, 'teams', teamName, 'inboxes', 'alice.json'); + const ownerInbox = JSON.parse(fs.readFileSync(ownerInboxPath, 'utf8')); + expect(ownerInbox.at(-1).summary).toContain(`#${createdTask.displayId}`); + expect(ownerInbox.at(-1).text).toContain('Need one more check'); + const attachment = parseJsonToolResult( await getTool('task_attach_comment_file').execute({ claudeDir, diff --git a/src/main/index.ts b/src/main/index.ts index a7764d21..c4864801 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -281,6 +281,7 @@ async function notifyNewSentMessages(teamName: string): Promise { for (let i = 0; i < newMessages.length; i++) { const msg = newMessages[i]; + if ((msg.to ?? '').trim() !== 'user') continue; // Skip messages sent from our own UI if (msg.source && suppressedSources.has(msg.source)) continue; // Skip internal coordination noise @@ -454,8 +455,7 @@ function wireFileWatcherEvents(context: ServiceContext): void { ); } - // Auto-relay ONLY lead-inbox changes into the live lead process. - // (Relaying on *any* inbox change causes the lead to process irrelevant status noise.) + // Relay inbox changes into active runtime recipients. if (teamProvisioningService.isTeamAlive(teamName) && detail.startsWith('inboxes/')) { const match = /^inboxes\/(.+)\.json$/.exec(detail); if (match && teamDataService) { @@ -463,8 +463,11 @@ function wireFileWatcherEvents(context: ServiceContext): void { void teamDataService .getLeadMemberName(teamName) .then((leadName) => { - if (!leadName || inboxName !== leadName) return; - return teamProvisioningService.relayLeadInboxMessages(teamName); + if (!leadName) return; + if (inboxName === leadName) { + return teamProvisioningService.relayLeadInboxMessages(teamName); + } + return teamProvisioningService.relayMemberInboxMessages(teamName, inboxName); }) .catch((e: unknown) => logger.warn(`[FileWatcher] relay failed for ${teamName}: ${String(e)}`) diff --git a/src/main/ipc/teams.ts b/src/main/ipc/teams.ts index 215cb62e..26328edc 100644 --- a/src/main/ipc/teams.ts +++ b/src/main/ipc/teams.ts @@ -1143,13 +1143,12 @@ async function handleSendMessage( from: payload.from, }); - // Best-effort: if team is alive and recipient is a teammate (not lead), - // also forward via the live lead process so in-process teammates receive it. + // Best-effort live relay so active processes see the inbox row promptly. if (!isLeadRecipient && isAlive) { try { - await provisioning.forwardUserDmToTeammate(tn, memberName, baseText, payload.summary); + await provisioning.relayMemberInboxMessages(tn, memberName); } catch (e: unknown) { - logger.warn(`Failed to forward user DM to teammate "${memberName}" via lead: ${String(e)}`); + logger.warn(`Relay after sendMessage failed for teammate "${memberName}": ${String(e)}`); } } diff --git a/src/main/services/team/TeamDataService.ts b/src/main/services/team/TeamDataService.ts index 076c2321..9bcaa825 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -954,58 +954,6 @@ export class TeamDataService { ...(attachments && attachments.length > 0 ? { attachments } : {}), } as TaskComment); - try { - const [tasks, config] = await Promise.all([ - this.taskReader.getTasks(teamName), - this.configReader.getConfig(teamName).catch(() => null), - ]); - const task = addResult.task ?? tasks.find((t) => t.id === taskId); - const leadName = this.resolveLeadNameFromConfig(config); - const owner = task?.owner?.trim() || null; - // Auto-clear needsClarification: "user" on UI comment - // UI comments always have author "user" (TeamTaskWriter default) - if (task?.needsClarification === 'user') { - controller.tasks.setNeedsClarification(taskId, null); - } - - if (task && owner && !this.isLeadOwner(owner, leadName)) { - // Notify non-lead task owner via inbox (lead → member message) - const parts = [ - `Comment on task ${this.getTaskLabel(task)} "${task.subject}":\n\n${text}`, - `\n${AGENT_BLOCK_OPEN}`, - `Reply to this comment using MCP tool task_add_comment:`, - `{ teamName: "${teamName}", taskId: "${taskId}", text: "", from: "" }`, - AGENT_BLOCK_CLOSE, - ]; - await this.sendMessage(teamName, { - member: owner, - from: leadName, - text: parts.join('\n'), - summary: `Comment on ${this.getTaskLabel(task)}`, - source: 'system_notification', - }); - } else if (task && owner && this.isLeadOwner(owner, leadName)) { - // Notify lead about user's comment on their own task. - // Write to lead's inbox — relay delivers to stdin when process is alive. - const parts = [ - `New comment from user on your task ${this.getTaskLabel(task)} "${task.subject}":\n\n${text}`, - `\n${AGENT_BLOCK_OPEN}`, - `Reply to this comment using MCP tool task_add_comment:`, - `{ teamName: "${teamName}", taskId: "${taskId}", text: "", from: "${leadName}" }`, - AGENT_BLOCK_CLOSE, - ]; - await this.sendMessage(teamName, { - member: leadName, - from: 'user', - text: parts.join('\n'), - summary: `Comment on ${this.getTaskLabel(task)}`, - source: 'system_notification', - }); - } - } catch { - // Notification is best-effort — don't fail comment save - } - return comment; } diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index d00e018c..a2c94df6 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -394,15 +394,17 @@ function buildTaskStatusProtocol(teamName: string): string { a) Owner finishes work on #X -> task_complete #X b) Reviewer accepts -> review_approve #X 11. CLARIFICATION PROTOCOL (CRITICAL — MANDATORY): - When you are blocked and need information to continue a task, you MUST do BOTH steps below — skipping the MCP update breaks the task board: - a) STEP 1 — FIRST, set the clarification flag with MCP tool task_set_clarification: - { teamName: "${teamName}", taskId: "", value: "lead" } - b) STEP 2 — THEN, send a message to your team lead via SendMessage explaining what you need. - IMPORTANT: Always update the task board BEFORE sending the message. The flag is what makes the task board show "needs clarification" — without it, your request is invisible on the board. - c) The flag is auto-cleared when the lead adds a task comment on your task. - If the lead replies via SendMessage instead, clear the flag yourself once you have the answer: - { teamName: "${teamName}", taskId: "", value: "clear" } - d) Do NOT set clarification to "user" yourself — only the team lead escalates to the user. + When you are blocked and need information to continue a task, you MUST do ALL steps below — skipping the board update or comment breaks traceability: + a) STEP 1 — FIRST, set the clarification flag with MCP tool task_set_clarification: + { teamName: "${teamName}", taskId: "", value: "lead" } + b) STEP 2 — THEN, add a task comment describing exactly what you need: + { teamName: "${teamName}", taskId: "", text: "question / blocker / missing info", from: "" } + c) STEP 3 — THEN, send a message to your team lead via SendMessage so they notice it promptly. + IMPORTANT: Always update the task board BEFORE sending the message. The flag + task comment are what make the request durable and visible on the board. + d) The flag is auto-cleared when the lead adds a task comment on your task. + If the lead replies via SendMessage instead, clear the flag yourself once you have the answer: + { teamName: "${teamName}", taskId: "", value: "clear" } + e) Do NOT set clarification to "user" yourself — only the team lead escalates to the user. 12. DEPENDENCY AWARENESS: When your task has blockedBy dependencies, check if they are completed before starting. When you complete a task that blocks others, mention this in your completion message so blocked teammates can proceed. @@ -484,7 +486,7 @@ function buildTeamCtlOpsInstructions(teamName: string, leadName: string): string `- Task assignment notifications are handled by the board runtime, so do NOT send a separate SendMessage for the same assignment unless you have extra context that is not already on the task.`, ``, `Clarification handling (CRITICAL — MANDATORY for correct task board state):`, - `- When a teammate needs clarification (needsClarification: "lead"), reply via task comment (preferred — auto-clears the flag) or SendMessage.`, + `- When a teammate needs clarification (needsClarification: "lead"), reply via task comment (preferred — auto-clears the flag and wakes the owner) or SendMessage.`, `- If you reply via SendMessage instead of task comment, also clear the flag manually:`, ` task_set_clarification { teamName: "${teamName}", taskId: "", value: "clear" }`, `- If you cannot answer and the user needs to decide — ESCALATION PROTOCOL:`, @@ -1000,6 +1002,8 @@ export class TeamProvisioningService { private readonly teamOpLocks = new Map>(); private readonly leadInboxRelayInFlight = new Map>(); private readonly relayedLeadInboxMessageIds = new Map>(); + private readonly memberInboxRelayInFlight = new Map>(); + private readonly relayedMemberInboxMessageIds = new Map>(); private readonly liveLeadProcessMessages = new Map(); private teamChangeEmitter: ((event: TeamChangeEvent) => void) | null = null; private helpOutputCache: string | null = null; @@ -1151,6 +1155,23 @@ export class TeamProvisioningService { } } + private getMemberRelayKey(teamName: string, memberName: string): string { + return `${teamName}:${memberName.trim()}`; + } + + private armSilentTeammateForward(run: ProvisioningRun, teammateName: string): void { + run.silentUserDmForward = { target: teammateName, startedAt: nowIso() }; + if (run.silentUserDmForwardClearHandle) { + clearTimeout(run.silentUserDmForwardClearHandle); + run.silentUserDmForwardClearHandle = null; + } + run.silentUserDmForwardClearHandle = setTimeout(() => { + run.silentUserDmForward = null; + run.silentUserDmForwardClearHandle = null; + }, 60_000); + run.silentUserDmForwardClearHandle.unref(); + } + private toolApprovalEventEmitter: ((event: ToolApprovalEvent) => void) | null = null; setToolApprovalEventEmitter(emitter: (event: ToolApprovalEvent) => void): void { @@ -2379,17 +2400,7 @@ export class TeamProvisioningService { return; } - run.silentUserDmForward = { target: teammateName, startedAt: nowIso() }; - if (run.silentUserDmForwardClearHandle) { - clearTimeout(run.silentUserDmForwardClearHandle); - run.silentUserDmForwardClearHandle = null; - } - // Safety valve: if the CLI never emits a result message, don't stay in "silent" mode forever. - run.silentUserDmForwardClearHandle = setTimeout(() => { - run.silentUserDmForward = null; - run.silentUserDmForwardClearHandle = null; - }, 60_000); - run.silentUserDmForwardClearHandle.unref(); + this.armSilentTeammateForward(run, teammateName); const summaryLine = userSummary?.trim() ? `Summary: ${userSummary.trim()}` : null; const internal = wrapInAgentBlock( @@ -2412,6 +2423,112 @@ export class TeamProvisioningService { await this.sendMessageToTeam(teamName, message); } + async relayMemberInboxMessages(teamName: string, memberName: string): Promise { + const relayKey = this.getMemberRelayKey(teamName, memberName); + const existing = this.memberInboxRelayInFlight.get(relayKey); + if (existing) { + return existing; + } + + const work = (async (): Promise => { + const runId = this.activeByTeam.get(teamName); + if (!runId) return 0; + const run = this.runs.get(runId); + if (!run?.child || run.processKilled || run.cancelRequested) return 0; + if (!run.provisioningComplete) return 0; + + const relayedIds = this.relayedMemberInboxMessageIds.get(relayKey) ?? new Set(); + + let memberInboxMessages: Awaited> = []; + try { + memberInboxMessages = await this.inboxReader.getMessagesFor(teamName, memberName); + } catch { + return 0; + } + + const unread = memberInboxMessages + .filter((m): m is InboxMessage & { messageId: string } => { + if (m.read) return false; + if (typeof m.text !== 'string' || m.text.trim().length === 0) return false; + if (!this.hasStableMessageId(m)) return false; + return !relayedIds.has(m.messageId); + }) + .sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)); + + if (unread.length === 0) return 0; + + const noiseUnread = unread.filter((m) => isInboxNoiseMessage(m.text)); + if (noiseUnread.length > 0) { + try { + await this.markInboxMessagesRead(teamName, memberName, noiseUnread); + } catch { + // best-effort + } + } + + const actionableUnread = unread.filter((m) => !isInboxNoiseMessage(m.text)); + if (actionableUnread.length === 0) return 0; + + const MAX_RELAY = 10; + const batch = actionableUnread.slice(0, MAX_RELAY); + + this.armSilentTeammateForward(run, memberName); + + const message = [ + `Relay inbox messages to teammate "${memberName}".`, + wrapInAgentBlock( + [ + `Use the SendMessage tool with recipient="${memberName}".`, + `Forward each inbox item below as a teammate message, preserving task IDs and critical instructions.`, + `Do NOT send any message to recipient "user" for this relay turn.`, + `Do NOT add extra narration outside the SendMessage calls.`, + ].join('\n') + ), + ``, + `Messages:`, + ...batch.flatMap((m, idx) => { + const summaryLine = m.summary?.trim() ? `Summary: ${m.summary.trim()}` : null; + return [ + `${idx + 1}) From: ${m.from || 'unknown'}`, + ` Timestamp: ${m.timestamp}`, + ...(summaryLine ? [` ${summaryLine}`] : []), + ` Text:`, + ...m.text.split('\n').map((line) => ` ${line}`), + ``, + ]; + }), + ].join('\n'); + + try { + await this.sendMessageToTeam(teamName, message); + } catch { + return 0; + } + + for (const m of batch) { + relayedIds.add(m.messageId); + } + this.relayedMemberInboxMessageIds.set(relayKey, this.trimRelayedSet(relayedIds)); + + try { + await this.markInboxMessagesRead(teamName, memberName, batch); + } catch { + // Best-effort: relay succeeded; marking read failed. + } + + return batch.length; + })(); + + this.memberInboxRelayInFlight.set(relayKey, work); + try { + return await work; + } finally { + if (this.memberInboxRelayInFlight.get(relayKey) === work) { + this.memberInboxRelayInFlight.delete(relayKey); + } + } + } + /** * Relay unread inbox messages addressed to the team lead into the live lead process. * @@ -2763,24 +2880,22 @@ export class TeamProvisioningService { } /** - * Intercept SendMessage(to: "user") tool_use blocks from the lead's stream-json output. + * Intercept SendMessage tool_use blocks from the lead's stream-json output. * * Claude Code's internal teamContext may be lost after session resume (--resume), causing - * SendMessage to route messages to ~/.claude/teams/default/ instead of the real team. - * By capturing tool_use calls directly from stdout, we persist them to sentMessages.json - * under the correct team name — ensuring the UI and OS notifications work correctly - * regardless of the internal teamContext state. + * SendMessage routing to drift away from our canonical team artifacts. By capturing tool_use + * calls directly from stdout, we persist a durable message row under the correct team name so + * Messages stays accurate even if Claude's own routing is flaky. */ - private captureSendMessageToUser(run: ProvisioningRun, content: Record[]): void { + private captureSendMessages(run: ProvisioningRun, content: Record[]): void { for (const part of content) { if (part.type !== 'tool_use' || part.name !== 'SendMessage') continue; const input = part.input; if (!input || typeof input !== 'object') continue; const inp = input as Record; - // Only capture messages addressed to the human user const recipient = typeof inp.recipient === 'string' ? inp.recipient : ''; - if (recipient !== 'user') continue; + if (!recipient.trim()) continue; const msgContent = typeof inp.content === 'string' ? inp.content : ''; if (msgContent.trim().length === 0) continue; @@ -2795,10 +2910,10 @@ export class TeamProvisioningService { const msg: InboxMessage = { from: leadName, - to: 'user', + to: recipient, text: cleanContent, timestamp: nowIso(), - read: false, + read: recipient !== 'user', summary: (summary || cleanContent).length > 60 ? (summary || cleanContent).slice(0, 57) + '...' @@ -2816,7 +2931,7 @@ export class TeamProvisioningService { }); logger.debug( - `[${run.teamName}] Captured SendMessage→user from stdout: ${cleanContent.slice(0, 100)}` + `[${run.teamName}] Captured SendMessage→${recipient} from stdout: ${cleanContent.slice(0, 100)}` ); } } @@ -2947,12 +3062,13 @@ export class TeamProvisioningService { return Array.isArray(inner) ? (inner as Record[]) : null; })(); - const hasSendMessageToUser = (content ?? []).some((part) => { + const hasCapturedSendMessage = (content ?? []).some((part) => { if (!part || typeof part !== 'object') return false; if (part.type !== 'tool_use' || part.name !== 'SendMessage') return false; const input = part.input; if (!input || typeof input !== 'object') return false; - return (input as Record).recipient === 'user'; + const recipient = (input as Record).recipient; + return typeof recipient === 'string' && recipient.trim().length > 0; }); const textParts = (content ?? []) @@ -2988,12 +3104,12 @@ export class TeamProvisioningService { } } else if (run.provisioningComplete) { // 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. + // When the same assistant message includes SendMessage(...), skip text — + // captureSendMessages() handles the visible outbound message separately. if ( !run.silentUserDmForward && !run.suppressPostCompactReminderOutput && - !hasSendMessageToUser + !hasCapturedSendMessage ) { const cleanText = stripAgentBlocks(text).trim(); if (cleanText.length > 0) { @@ -3003,7 +3119,7 @@ export class TeamProvisioningService { } else { // Pre-ready: also push to live cache so Messages shows early narration // once team:getData becomes readable. The banner still uses provisioningOutputParts. - if (!run.silentUserDmForward && !hasSendMessageToUser) { + if (!run.silentUserDmForward && !hasCapturedSendMessage) { const cleanText = stripAgentBlocks(text).trim(); if (cleanText.length > 0) { this.pushLiveLeadTextMessage(run, cleanText); @@ -3029,14 +3145,11 @@ export class TeamProvisioningService { } } - // Capture SendMessage(to: "user") tool_use blocks from assistant output. - // Claude Code's internal teamContext may route to "default" instead of the real team - // (e.g., after session resume when teamContext is lost). We intercept the tool calls - // from stdout and persist them to sentMessages.json under the correct team name, - // ensuring the UI and notifications show the right team. - // Works in both pre-ready and post-ready phases so provisioning-time user DMs are captured. + // Capture SendMessage tool_use blocks from assistant output. + // Works in both pre-ready and post-ready phases so outbound runtime messages + // are visible in our team message artifacts even if Claude's own routing drifts. if (!run.silentUserDmForward) { - this.captureSendMessageToUser(run, content ?? []); + this.captureSendMessages(run, content ?? []); } // Extract context window usage from message.usage for real-time tracking. @@ -3947,6 +4060,16 @@ export class TeamProvisioningService { this.activeByTeam.delete(run.teamName); this.leadInboxRelayInFlight.delete(run.teamName); this.relayedLeadInboxMessageIds.delete(run.teamName); + for (const key of Array.from(this.memberInboxRelayInFlight.keys())) { + if (key.startsWith(`${run.teamName}:`)) { + this.memberInboxRelayInFlight.delete(key); + } + } + for (const key of Array.from(this.relayedMemberInboxMessageIds.keys())) { + if (key.startsWith(`${run.teamName}:`)) { + this.relayedMemberInboxMessageIds.delete(key); + } + } this.liveLeadProcessMessages.delete(run.teamName); // Dismiss any pending tool approvals for this run if (run.pendingApprovals.size > 0) { diff --git a/src/renderer/components/team/UnreadCommentsBadge.tsx b/src/renderer/components/team/UnreadCommentsBadge.tsx index de26cf87..9df735a2 100644 --- a/src/renderer/components/team/UnreadCommentsBadge.tsx +++ b/src/renderer/components/team/UnreadCommentsBadge.tsx @@ -12,11 +12,13 @@ export const UnreadCommentsBadge = ({ if (totalCount === 0) return null; return ( - + 0 ? 'mr-1 pl-1.5 pr-2' : 'px-1.5'}`} + > {totalCount} {unreadCount > 0 && ( - + {unreadCount} )} diff --git a/src/renderer/components/team/activity/ActivityTimeline.tsx b/src/renderer/components/team/activity/ActivityTimeline.tsx index 227366b0..3931b6d9 100644 --- a/src/renderer/components/team/activity/ActivityTimeline.tsx +++ b/src/renderer/components/team/activity/ActivityTimeline.tsx @@ -357,7 +357,7 @@ export const ActivityTimeline = ({ sessionSeparator = (
diff --git a/src/renderer/components/team/activity/LeadThoughtsGroup.tsx b/src/renderer/components/team/activity/LeadThoughtsGroup.tsx index 83025af7..af7e8a60 100644 --- a/src/renderer/components/team/activity/LeadThoughtsGroup.tsx +++ b/src/renderer/components/team/activity/LeadThoughtsGroup.tsx @@ -39,6 +39,7 @@ export interface LeadThoughtGroup { * an official message (SendMessage, direct reply, inbox, etc.). */ export function isLeadThought(msg: InboxMessage): boolean { + if (typeof msg.to === 'string' && msg.to.trim().length > 0) return false; if (msg.source === 'lead_session') return true; if (msg.source === 'lead_process') return true; return false; @@ -362,7 +363,10 @@ const LeadThoughtItem = ({
)}
-
+
{ e.stopPropagation(); @@ -370,7 +370,7 @@ export const KanbanTaskCard = ({
); } - return filtered.map((m) => { + const sorted = [...filtered].sort((a, b) => { + const aIsLead = a.role === 'lead' || a.name === 'team-lead' ? 1 : 0; + const bIsLead = b.role === 'lead' || b.name === 'team-lead' ? 1 : 0; + return bIsLead - aIsLead; + }); + return sorted.map((m) => { const resolvedColor = colorMap.get(m.name); const role = formatAgentRole(m.role) ?? formatAgentRole(m.agentType); const isSelected = m.name === recipient; diff --git a/src/renderer/components/team/review/CodeMirrorDiffView.tsx b/src/renderer/components/team/review/CodeMirrorDiffView.tsx index 781bd2b5..61664849 100644 --- a/src/renderer/components/team/review/CodeMirrorDiffView.tsx +++ b/src/renderer/components/team/review/CodeMirrorDiffView.tsx @@ -140,9 +140,9 @@ const diffSpecificTheme = EditorView.theme({ fontSize: '12px', fontWeight: '500', lineHeight: '20px', - color: 'var(--color-text)', - backgroundColor: 'var(--color-surface-raised)', - border: '1px solid var(--color-border)', + color: 'var(--diff-merge-undo-color)', + backgroundColor: 'var(--diff-merge-undo-bg)', + border: '1px solid var(--diff-merge-undo-border)', '&:hover': { backgroundColor: 'var(--diff-merge-undo-hover-bg)' }, '& kbd': { fontSize: '10px', color: 'var(--color-text-muted)', marginLeft: '4px' }, }, diff --git a/src/renderer/components/team/review/ReviewDiffContent.tsx b/src/renderer/components/team/review/ReviewDiffContent.tsx index d5f775f9..4cd22f27 100644 --- a/src/renderer/components/team/review/ReviewDiffContent.tsx +++ b/src/renderer/components/team/review/ReviewDiffContent.tsx @@ -140,7 +140,7 @@ export const ReviewDiffContent = ({ file }: ReviewDiffContentProps) => {
{nonErrorSnippets.map((snippet, index) => ( { sendMessageToTeam: vi.fn(async () => undefined), isTeamAlive: vi.fn(() => true), relayLeadInboxMessages: vi.fn(async () => 0), + relayMemberInboxMessages: vi.fn(async () => 0), getLiveLeadProcessMessages: vi.fn(() => [] as InboxMessage[]), getAliveTeams: vi.fn(() => ['my-team']), getLeadActivityState: vi.fn(() => 'idle'), diff --git a/test/main/services/team/TeamProvisioningServiceLiveMessages.test.ts b/test/main/services/team/TeamProvisioningServiceLiveMessages.test.ts index 514c914d..06f2f8f4 100644 --- a/test/main/services/team/TeamProvisioningServiceLiveMessages.test.ts +++ b/test/main/services/team/TeamProvisioningServiceLiveMessages.test.ts @@ -274,6 +274,36 @@ describe('TeamProvisioningService pre-ready live messages', () => { expect(hoisted.appendSentMessage).toHaveBeenCalledTimes(1); }); + it('captures SendMessage(to:team-lead) without rendering duplicate assistant thought text', () => { + const service = new TeamProvisioningService(); + seedConfig('my-team'); + const run = attachRun(service, 'my-team', { provisioningComplete: true }); + + callHandleStreamJsonMessage(service, run, { + type: 'assistant', + content: [ + { type: 'text', text: 'Forwarding the clarification request now.' }, + { + type: 'tool_use', + name: 'SendMessage', + input: { + type: 'message', + recipient: 'team-lead', + content: 'Need clarification on #abcd1234', + summary: 'Clarification request', + }, + }, + ], + }); + + const live = service.getLiveLeadProcessMessages('my-team'); + expect(live).toHaveLength(1); + expect(live[0].to).toBe('team-lead'); + expect(live[0].text).toBe('Need clarification on #abcd1234'); + expect(live[0].source).toBe('lead_process'); + expect(hoisted.appendSentMessage).toHaveBeenCalledTimes(1); + }); + it('post-ready path also uses the unified helper', () => { const service = new TeamProvisioningService(); seedConfig('my-team'); diff --git a/test/main/services/team/TeamProvisioningServicePrompts.test.ts b/test/main/services/team/TeamProvisioningServicePrompts.test.ts index 3a5223e7..95912b7c 100644 --- a/test/main/services/team/TeamProvisioningServicePrompts.test.ts +++ b/test/main/services/team/TeamProvisioningServicePrompts.test.ts @@ -216,6 +216,9 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(prompt).toContain(` ${AGENT_BLOCK_OPEN}`); expect(prompt).toContain(` ${AGENT_BLOCK_CLOSE}`); expect(prompt).toContain('NEVER use agent-only blocks in messages to "user".'); + expect(prompt).toContain('you MUST do ALL steps below'); + expect(prompt).toContain('STEP 2 — THEN, add a task comment describing exactly what you need'); + expect(prompt).toContain('STEP 3 — THEN, send a message to your team lead via SendMessage'); expect(prompt).toContain('use task_briefing as your compact queue view'); expect(prompt).toContain('Use task_get when you need the full task context before starting a pending/needsFix task'); expect(prompt).toContain('Use task_briefing as a compact queue view of your assigned tasks.'); @@ -278,6 +281,7 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(prompt).toContain(` ${AGENT_BLOCK_OPEN}`); expect(prompt).toContain(` ${AGENT_BLOCK_CLOSE}`); expect(prompt).toContain('NEVER use agent-only blocks in messages to "user".'); + expect(prompt).toContain('reply via task comment (preferred — auto-clears the flag and wakes the owner) or SendMessage'); expect(prompt).toContain('Your FIRST action: call MCP tool task_briefing'); expect(prompt).toContain('resume/finish those first'); expect(prompt).toContain('Call task_get only if you need more context than task_briefing already gave you'); diff --git a/test/main/services/team/TeamProvisioningServiceRelay.test.ts b/test/main/services/team/TeamProvisioningServiceRelay.test.ts index 76146444..f0771c04 100644 --- a/test/main/services/team/TeamProvisioningServiceRelay.test.ts +++ b/test/main/services/team/TeamProvisioningServiceRelay.test.ts @@ -105,6 +105,10 @@ function seedLeadInbox(teamName: string, messages: unknown[]): void { hoisted.files.set(`/mock/teams/${teamName}/inboxes/team-lead.json`, JSON.stringify(messages)); } +function seedMemberInbox(teamName: string, memberName: string, messages: unknown[]): void { + hoisted.files.set(`/mock/teams/${teamName}/inboxes/${memberName}.json`, JSON.stringify(messages)); +} + function attachAliveRun( service: TeamProvisioningService, teamName: string, @@ -293,4 +297,30 @@ describe('TeamProvisioningService relayLeadInboxMessages', () => { expect(writeSpy).toHaveBeenCalledTimes(0); expect(hoisted.appendSentMessage).not.toHaveBeenCalled(); }); + + it('relays unread teammate inbox messages through the live team process', async () => { + const service = new TeamProvisioningService(); + const teamName = 'my-team'; + seedConfig(teamName); + seedMemberInbox(teamName, 'alice', [ + { + from: 'team-lead', + text: 'Comment on task #abcd1234 "Investigate":\n\nPlease retry with logging enabled.', + timestamp: '2026-02-23T10:00:00.000Z', + read: false, + summary: 'Comment on #abcd1234', + messageId: 'm-alice-1', + }, + ]); + + const { writeSpy } = attachAliveRun(service, teamName); + const relayed = await service.relayMemberInboxMessages(teamName, 'alice'); + + expect(relayed).toBe(1); + expect(writeSpy).toHaveBeenCalledTimes(1); + const payload = String(writeSpy.mock.calls[0]?.[0] ?? ''); + expect(payload).toContain('"type":"user"'); + expect(payload).toContain('recipient=\\"alice\\"'); + expect(payload).toContain('Please retry with logging enabled.'); + }); }); diff --git a/test/renderer/components/team/activity/LeadThoughtsGroup.test.ts b/test/renderer/components/team/activity/LeadThoughtsGroup.test.ts new file mode 100644 index 00000000..4663d981 --- /dev/null +++ b/test/renderer/components/team/activity/LeadThoughtsGroup.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest'; + +import { isLeadThought } from '../../../../../src/renderer/components/team/activity/LeadThoughtsGroup'; + +describe('LeadThoughtsGroup', () => { + it('does not classify outbound runtime messages with recipients as lead thoughts', () => { + expect( + isLeadThought({ + from: 'team-lead', + to: 'alice', + text: 'Please check task #abcd1234', + timestamp: '2026-03-08T00:00:00.000Z', + read: true, + source: 'lead_process', + }) + ).toBe(false); + }); +});