diff --git a/agent-teams-controller/src/internal/context.js b/agent-teams-controller/src/internal/context.js index 94e720fe..abe218e1 100644 --- a/agent-teams-controller/src/internal/context.js +++ b/agent-teams-controller/src/internal/context.js @@ -1,4 +1,4 @@ -const legacy = require('../legacy/teamctl.cli.js'); +const runtimeHelpers = require('./runtimeHelpers.js'); function createControllerContext(options = {}) { const teamName = String(options.teamName || '').trim(); @@ -11,7 +11,7 @@ function createControllerContext(options = {}) { flags['claude-dir'] = options.claudeDir.trim(); } - const paths = legacy.getPaths(flags, teamName); + const paths = runtimeHelpers.getPaths(flags, teamName); return { teamName, claudeDir: paths.claudeDir, diff --git a/agent-teams-controller/src/internal/processStore.js b/agent-teams-controller/src/internal/processStore.js index b65d4cb0..f2af6245 100644 --- a/agent-teams-controller/src/internal/processStore.js +++ b/agent-teams-controller/src/internal/processStore.js @@ -2,7 +2,7 @@ const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); -const legacy = require('../legacy/teamctl.cli.js'); +const runtimeHelpers = require('./runtimeHelpers.js'); function nowIso() { return new Date().toISOString(); @@ -39,7 +39,7 @@ function listProcesses(paths) { const alive = !entry.stoppedAt && Number.isFinite(Number(entry.pid)) && - legacy.isProcessAlive(Number(entry.pid)); + runtimeHelpers.isProcessAlive(Number(entry.pid)); if (!alive && !entry.stoppedAt) { return { @@ -83,7 +83,7 @@ function registerProcess(paths, flags) { existingActiveIndex >= 0 ? { ...list[existingActiveIndex], - ...(legacy.isProcessAlive(pid) ? {} : { stoppedAt: nowIso() }), + ...(runtimeHelpers.isProcessAlive(pid) ? {} : { stoppedAt: nowIso() }), } : null; if (existingActiveIndex >= 0 && existingActive && existingActive.stoppedAt) { diff --git a/agent-teams-controller/src/internal/review.js b/agent-teams-controller/src/internal/review.js index 3599d7b2..a42dc2f2 100644 --- a/agent-teams-controller/src/internal/review.js +++ b/agent-teams-controller/src/internal/review.js @@ -1,3 +1,6 @@ +const fs = require('fs'); +const path = require('path'); + const kanban = require('./kanban.js'); const messages = require('./messages.js'); const tasks = require('./tasks.js'); @@ -13,6 +16,22 @@ function getReviewer(context, flags) { : null; } +function resolveLeadSessionId(context, flags) { + if (typeof flags.leadSessionId === 'string' && flags.leadSessionId.trim()) { + return flags.leadSessionId.trim(); + } + + try { + const configPath = path.join(context.paths.teamDir, 'config.json'); + const parsed = JSON.parse(fs.readFileSync(configPath, 'utf8')); + return typeof parsed.leadSessionId === 'string' && parsed.leadSessionId.trim() + ? parsed.leadSessionId.trim() + : undefined; + } catch { + return undefined; + } +} + function requestReview(context, taskId, flags = {}) { const task = tasks.getTask(context, taskId); if (task.status !== 'completed') { @@ -22,6 +41,7 @@ function requestReview(context, taskId, flags = {}) { const from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'team-lead'; const reviewer = getReviewer(context, flags); + const leadSessionId = resolveLeadSessionId(context, flags); try { kanban.setKanbanColumn(context, task.id, 'review'); @@ -42,6 +62,7 @@ function requestReview(context, taskId, flags = {}) { ), summary: `Review request for #${task.displayId || task.id}`, source: 'system_notification', + ...(leadSessionId ? { leadSessionId } : {}), }); return tasks.getTask(context, task.id); } catch (error) { @@ -59,6 +80,7 @@ function approveReview(context, taskId, flags = {}) { const from = typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() : 'team-lead'; const note = typeof flags.note === 'string' && flags.note.trim() ? flags.note.trim() : 'Approved'; + const leadSessionId = resolveLeadSessionId(context, flags); kanban.setKanbanColumn(context, task.id, 'approved'); tasks.addTaskComment(context, task.id, { @@ -77,6 +99,7 @@ function approveReview(context, taskId, flags = {}) { : `Task ${task.displayId || task.id} approved.`, summary: `Approved ${task.displayId || task.id}`, source: 'system_notification', + ...(leadSessionId ? { leadSessionId } : {}), }); } @@ -95,6 +118,7 @@ function requestChanges(context, taskId, flags = {}) { typeof flags.comment === 'string' && flags.comment.trim() ? flags.comment.trim() : 'Reviewer requested changes.'; + const leadSessionId = resolveLeadSessionId(context, flags); kanban.clearKanban(context, task.id); tasks.setTaskStatus(context, task.id, 'in_progress', from); @@ -111,6 +135,7 @@ function requestChanges(context, taskId, flags = {}) { 'Please fix and mark it as completed when ready.', summary: `Fix request for ${task.displayId || task.id}`, source: 'system_notification', + ...(leadSessionId ? { leadSessionId } : {}), }); return tasks.getTask(context, task.id); diff --git a/agent-teams-controller/src/internal/runtimeHelpers.js b/agent-teams-controller/src/internal/runtimeHelpers.js new file mode 100644 index 00000000..2c058cbf --- /dev/null +++ b/agent-teams-controller/src/internal/runtimeHelpers.js @@ -0,0 +1,295 @@ +const fs = require('fs'); +const path = require('path'); +const crypto = require('crypto'); + +const TASK_ATTACHMENTS_DIR = 'task-attachments'; +const MAX_TASK_ATTACHMENT_BYTES = 20 * 1024 * 1024; + +function nowIso() { + return new Date().toISOString(); +} + +function makeId() { + return crypto.randomUUID ? crypto.randomUUID() : `${Date.now()}-${Math.random()}`; +} + +function ensureDir(dirPath) { + fs.mkdirSync(dirPath, { recursive: true }); +} + +function readJson(filePath, fallbackValue) { + try { + return JSON.parse(fs.readFileSync(filePath, 'utf8')); + } catch (error) { + if (error && error.code === 'ENOENT') { + return fallbackValue; + } + throw error; + } +} + +function isSafePathSegment(value) { + const normalized = String(value == null ? '' : value); + if (normalized.length === 0 || normalized.trim().length === 0) return false; + if (normalized === '.' || normalized === '..') return false; + if (normalized.includes('/') || normalized.includes('\\')) return false; + if (normalized.includes('..')) return false; + if (normalized.includes('\0')) return false; + return true; +} + +function assertSafePathSegment(label, value) { + const normalized = String(value == null ? '' : value); + if (!isSafePathSegment(normalized)) { + throw new Error(`Invalid ${String(label)}`); + } + return normalized; +} + +function getHomeDir() { + if (process.env.HOME) return process.env.HOME; + if (process.env.USERPROFILE) return process.env.USERPROFILE; + if (process.env.HOMEDRIVE && process.env.HOMEPATH) { + return process.env.HOMEDRIVE + process.env.HOMEPATH; + } + try { + return require('os').homedir(); + } catch { + return ''; + } +} + +function getClaudeDir(flags) { + const explicit = + (typeof flags['claude-dir'] === 'string' && flags['claude-dir']) || + (typeof flags.claudeDir === 'string' && flags.claudeDir) || + (typeof flags.claude_path === 'string' && flags.claude_path) || + ''; + if (explicit) { + return path.resolve(explicit); + } + const home = getHomeDir(); + if (!home) { + throw new Error('HOME/USERPROFILE is not set'); + } + return path.join(home, '.claude'); +} + +function getPaths(flags, teamName) { + const claudeDir = getClaudeDir(flags); + const safeTeam = assertSafePathSegment('team', teamName); + const teamDir = path.join(claudeDir, 'teams', safeTeam); + const tasksDir = path.join(claudeDir, 'tasks', safeTeam); + const kanbanPath = path.join(teamDir, 'kanban-state.json'); + const processesPath = path.join(teamDir, 'processes.json'); + return { claudeDir, teamDir, tasksDir, kanbanPath, processesPath }; +} + +function inferLeadName(paths) { + const config = readJson(path.join(paths.teamDir, 'config.json'), null); + if (!config || !Array.isArray(config.members)) { + return 'team-lead'; + } + const lead = config.members.find( + (member) => member && member.role && String(member.role).toLowerCase().includes('lead') + ); + if (lead) { + return String(lead.name); + } + return config.members[0] ? String(config.members[0].name) : 'team-lead'; +} + +function isProcessAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if (error && error.code === 'EPERM') { + return true; + } + return false; + } +} + +function sanitizeFilename(original) { + const raw = String(original == null ? '' : original).trim(); + const parts = raw.split(/[\\/]/); + const base = (parts.length ? parts[parts.length - 1] : raw).trim(); + const cleaned = base + .replace(/\0/g, '') + .replace(/[\r\n\t]/g, ' ') + .replace(/[\\/]/g, '_') + .trim(); + if (!cleaned) return 'attachment'; + return cleaned.length > 180 ? cleaned.slice(0, 180) : cleaned; +} + +function readFileHeader(filePath, maxBytes) { + const fd = fs.openSync(filePath, 'r'); + try { + const buffer = Buffer.alloc(maxBytes); + const bytes = fs.readSync(fd, buffer, 0, maxBytes, 0); + return buffer.slice(0, bytes); + } finally { + try { + fs.closeSync(fd); + } catch { + // ignore + } + } +} + +function startsWithBytes(buffer, bytes) { + if (!buffer || buffer.length < bytes.length) return false; + for (let i = 0; i < bytes.length; i += 1) { + if (buffer[i] !== bytes[i]) return false; + } + return true; +} + +function detectMimeTypeFromPathAndHeader(filePath, filename) { + const name = String(filename || '').toLowerCase(); + const ext = path.extname(name); + + if (ext === '.png') return 'image/png'; + if (ext === '.jpg' || ext === '.jpeg') return 'image/jpeg'; + if (ext === '.gif') return 'image/gif'; + if (ext === '.webp') return 'image/webp'; + if (ext === '.pdf') return 'application/pdf'; + if (ext === '.txt') return 'text/plain'; + if (ext === '.md') return 'text/markdown'; + if (ext === '.json') return 'application/json'; + if (ext === '.zip') return 'application/zip'; + + let header; + try { + header = readFileHeader(filePath, 16); + } catch { + return 'application/octet-stream'; + } + + if (startsWithBytes(header, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) return 'image/png'; + if (startsWithBytes(header, [0xff, 0xd8, 0xff])) return 'image/jpeg'; + if (header.length >= 6) { + const signature6 = header.slice(0, 6).toString('ascii'); + if (signature6 === 'GIF87a' || signature6 === 'GIF89a') return 'image/gif'; + } + if (header.length >= 12) { + const riff = header.slice(0, 4).toString('ascii'); + const webp = header.slice(8, 12).toString('ascii'); + if (riff === 'RIFF' && webp === 'WEBP') return 'image/webp'; + } + if (header.length >= 5 && header.slice(0, 5).toString('ascii') === '%PDF-') { + return 'application/pdf'; + } + if (startsWithBytes(header, [0x50, 0x4b, 0x03, 0x04])) return 'application/zip'; + + return 'application/octet-stream'; +} + +function getTaskAttachmentsDir(paths, taskId) { + const safeTaskId = assertSafePathSegment('taskId', taskId); + return path.join(paths.teamDir, TASK_ATTACHMENTS_DIR, safeTaskId); +} + +function getStoredAttachmentPath(paths, taskId, attachmentId, filename) { + const safeFilename = sanitizeFilename(filename); + return path.join( + getTaskAttachmentsDir(paths, taskId), + `${String(attachmentId)}--${safeFilename}` + ); +} + +function ensureSourceFileReadable(srcPath) { + const stats = fs.statSync(srcPath); + if (!stats.isFile()) { + throw new Error(`Not a file: ${String(srcPath)}`); + } + if (stats.size > MAX_TASK_ATTACHMENT_BYTES) { + throw new Error( + `Attachment too large: ${(stats.size / (1024 * 1024)).toFixed(1)} MB (max ${String( + MAX_TASK_ATTACHMENT_BYTES / (1024 * 1024) + )} MB)` + ); + } + return stats; +} + +function copyOrLinkFile(srcPath, destPath, mode, allowFallback) { + const normalizedMode = String(mode || 'copy').toLowerCase(); + if (normalizedMode === 'link') { + try { + fs.linkSync(srcPath, destPath); + return { mode: 'link', fallbackUsed: false }; + } catch (error) { + if (!allowFallback) throw error; + try { + fs.copyFileSync(srcPath, destPath); + return { mode: 'copy', fallbackUsed: true }; + } catch (copyError) { + throw copyError || error; + } + } + } + + fs.copyFileSync(srcPath, destPath); + return { mode: 'copy', fallbackUsed: false }; +} + +function saveTaskAttachmentFile(paths, taskId, flags) { + const rawFile = + (typeof flags.file === 'string' && flags.file.trim()) || + (typeof flags.path === 'string' && flags.path.trim()) || + ''; + if (!rawFile) { + throw new Error('Missing --file '); + } + + const srcPath = path.resolve(rawFile); + ensureSourceFileReadable(srcPath); + + const filename = + (typeof flags.filename === 'string' && flags.filename.trim()) || path.basename(srcPath); + const mimeType = + (typeof flags['mime-type'] === 'string' && flags['mime-type'].trim()) || + (typeof flags.mimeType === 'string' && flags.mimeType.trim()) || + detectMimeTypeFromPathAndHeader(srcPath, filename); + + const attachmentId = makeId(); + const dir = getTaskAttachmentsDir(paths, taskId); + ensureDir(dir); + const destPath = getStoredAttachmentPath(paths, taskId, attachmentId, filename); + const allowFallback = !(flags['no-fallback'] === true); + + if (fs.existsSync(destPath)) { + throw new Error('Attachment destination already exists'); + } + + const result = copyOrLinkFile(srcPath, destPath, flags.mode, allowFallback); + const stats = fs.statSync(destPath); + if (!stats.isFile() || stats.size < 0) { + throw new Error('Attachment write verification failed'); + } + + const meta = { + id: attachmentId, + filename, + mimeType, + size: stats.size, + addedAt: nowIso(), + }; + + return { + meta, + storedPath: destPath, + storageMode: result.mode, + fallbackUsed: result.fallbackUsed, + }; +} + +module.exports = { + getPaths, + inferLeadName, + isProcessAlive, + saveTaskAttachmentFile, +}; diff --git a/agent-teams-controller/src/internal/tasks.js b/agent-teams-controller/src/internal/tasks.js index be02b124..0c67f4ed 100644 --- a/agent-teams-controller/src/internal/tasks.js +++ b/agent-teams-controller/src/internal/tasks.js @@ -1,5 +1,5 @@ -const legacy = require('../legacy/teamctl.cli.js'); const taskStore = require('./taskStore.js'); +const runtimeHelpers = require('./runtimeHelpers.js'); function createTask(context, input) { return taskStore.createTask(context.paths, input); @@ -56,7 +56,7 @@ function addTaskComment(context, taskId, flags) { author: typeof flags.from === 'string' && flags.from.trim() ? flags.from.trim() - : legacy.inferLeadName(context.paths), + : runtimeHelpers.inferLeadName(context.paths), ...(flags.id ? { id: flags.id } : {}), ...(flags.createdAt ? { createdAt: flags.createdAt } : {}), ...(flags.type ? { type: flags.type } : {}), @@ -75,7 +75,7 @@ function addTaskComment(context, taskId, flags) { function attachTaskFile(context, taskId, flags) { const canonicalTaskId = resolveTaskId(context, taskId); - const saved = legacy.saveTaskAttachmentFile(context.paths, canonicalTaskId, flags); + const saved = runtimeHelpers.saveTaskAttachmentFile(context.paths, canonicalTaskId, flags); const task = taskStore.addTaskAttachmentMeta(context.paths, canonicalTaskId, saved.meta); return { ...saved.meta, @@ -85,7 +85,7 @@ function attachTaskFile(context, taskId, flags) { function attachCommentFile(context, taskId, commentId, flags) { const canonicalTaskId = resolveTaskId(context, taskId); - const saved = legacy.saveTaskAttachmentFile(context.paths, canonicalTaskId, flags); + const saved = runtimeHelpers.saveTaskAttachmentFile(context.paths, canonicalTaskId, flags); const task = taskStore.addCommentAttachmentMeta(context.paths, canonicalTaskId, commentId, saved.meta); return { ...saved.meta, diff --git a/agent-teams-controller/test/controller.test.js b/agent-teams-controller/test/controller.test.js index 02772457..c6a2894a 100644 --- a/agent-teams-controller/test/controller.test.js +++ b/agent-teams-controller/test/controller.test.js @@ -14,6 +14,7 @@ describe('agent-teams-controller API', () => { JSON.stringify( { name: 'my-team', + leadSessionId: 'lead-session-1', members: [ { name: 'alice', role: 'team-lead' }, { name: 'bob', role: 'developer' }, @@ -68,6 +69,11 @@ describe('agent-teams-controller API', () => { }); expect(sent.leadSessionId).toBe('session-1'); + const ownerInboxPath = path.join(claudeDir, 'teams', 'my-team', 'inboxes', 'bob.json'); + const ownerInbox = JSON.parse(fs.readFileSync(ownerInboxPath, 'utf8')); + expect(ownerInbox.at(-1).summary).toContain('Approved'); + expect(ownerInbox.at(-1).leadSessionId).toBe('lead-session-1'); + const proc = controller.processes.registerProcess({ pid: process.pid, label: 'dev-server', @@ -250,6 +256,7 @@ describe('agent-teams-controller API', () => { expect(inbox[0].text).toContain(''); expect(inbox[0].text).toContain('review_approve'); expect(inbox[0].text).not.toContain(''); + expect(inbox[0].leadSessionId).toBe('lead-session-1'); }); it('persists full inbox metadata through controller messages.sendMessage', () => { @@ -297,6 +304,7 @@ describe('agent-teams-controller API', () => { const rows = JSON.parse(fs.readFileSync(inboxPath, 'utf8')); expect(rows.at(-1).source).toBe('system_notification'); expect(rows.at(-1).summary).toContain('Fix request'); + expect(rows.at(-1).leadSessionId).toBe('lead-session-1'); }); it('marks stale processes stopped during listing and supports unregister', () => { diff --git a/mcp-server/src/tools/reviewTools.ts b/mcp-server/src/tools/reviewTools.ts index 2d863d1b..08293ca4 100644 --- a/mcp-server/src/tools/reviewTools.ts +++ b/mcp-server/src/tools/reviewTools.ts @@ -18,13 +18,15 @@ export function registerReviewTools(server: Pick) { taskId: z.string().min(1), from: z.string().optional(), reviewer: z.string().optional(), + leadSessionId: z.string().optional(), }), - execute: async ({ teamName, claudeDir, taskId, from, reviewer }) => + execute: async ({ teamName, claudeDir, taskId, from, reviewer, leadSessionId }) => await Promise.resolve( jsonTextContent( getController(teamName, claudeDir).review.requestReview(taskId, { - ...(from ? { from } : {}), - ...(reviewer ? { reviewer } : {}), + ...(from ? { from } : {}), + ...(reviewer ? { reviewer } : {}), + ...(leadSessionId ? { leadSessionId } : {}), }) ) ), @@ -39,14 +41,16 @@ export function registerReviewTools(server: Pick) { from: z.string().optional(), note: z.string().optional(), notifyOwner: z.boolean().optional(), + leadSessionId: z.string().optional(), }), - execute: async ({ teamName, claudeDir, taskId, from, note, notifyOwner }) => + execute: async ({ teamName, claudeDir, taskId, from, note, notifyOwner, leadSessionId }) => await Promise.resolve( jsonTextContent( getController(teamName, claudeDir).review.approveReview(taskId, { - ...(from ? { from } : {}), - ...(note ? { note } : {}), - ...(notifyOwner !== false ? { 'notify-owner': true } : {}), + ...(from ? { from } : {}), + ...(note ? { note } : {}), + ...(notifyOwner !== false ? { 'notify-owner': true } : {}), + ...(leadSessionId ? { leadSessionId } : {}), }) ) ), @@ -60,13 +64,15 @@ export function registerReviewTools(server: Pick) { taskId: z.string().min(1), from: z.string().optional(), comment: z.string().optional(), + leadSessionId: z.string().optional(), }), - execute: async ({ teamName, claudeDir, taskId, from, comment }) => + execute: async ({ teamName, claudeDir, taskId, from, comment, leadSessionId }) => await Promise.resolve( jsonTextContent( getController(teamName, claudeDir).review.requestChanges(taskId, { - ...(from ? { from } : {}), - ...(comment ? { comment } : {}), + ...(from ? { from } : {}), + ...(comment ? { comment } : {}), + ...(leadSessionId ? { leadSessionId } : {}), }) ) ), diff --git a/mcp-server/test/tools.test.ts b/mcp-server/test/tools.test.ts index 19dacce3..ff9005c1 100644 --- a/mcp-server/test/tools.test.ts +++ b/mcp-server/test/tools.test.ts @@ -233,10 +233,14 @@ describe('agent-teams-mcp tools', () => { taskId: createdTask.id, from: 'lead', reviewer: 'alice', + leadSessionId: 'session-review-1', }) ); expect(reviewRequested.reviewState).toBe('review'); + const reviewerInboxPath = path.join(claudeDir, 'teams', teamName, 'inboxes', 'alice.json'); + const reviewerInbox = JSON.parse(fs.readFileSync(reviewerInboxPath, 'utf8')); + expect(reviewerInbox.at(-1).leadSessionId).toBe('session-review-1'); const approved = parseJsonToolResult( await getTool('review_approve').execute({ @@ -246,9 +250,13 @@ describe('agent-teams-mcp tools', () => { from: 'lead', note: 'Looks good', notifyOwner: true, + leadSessionId: 'session-review-1', }) ); expect(approved.reviewState).toBe('approved'); + const ownerInboxPath = path.join(claudeDir, 'teams', teamName, 'inboxes', 'alice.json'); + const ownerInbox = JSON.parse(fs.readFileSync(ownerInboxPath, 'utf8')); + expect(ownerInbox.at(-1).leadSessionId).toBe('session-review-1'); const kanbanState = parseJsonToolResult( await getTool('kanban_get').execute({ @@ -294,6 +302,7 @@ describe('agent-teams-mcp tools', () => { taskId: createdTask.id, from: 'lead', reviewer: 'alice', + leadSessionId: 'session-review-2', }); const changesRequested = parseJsonToolResult( @@ -303,11 +312,15 @@ describe('agent-teams-mcp tools', () => { taskId: createdTask.id, from: 'alice', comment: 'Please revise this section.', + leadSessionId: 'session-review-2', }) ); expect(changesRequested.status).toBe('in_progress'); expect(changesRequested.reviewState).toBe('none'); + const ownerInboxPath = path.join(claudeDir, 'teams', teamName, 'inboxes', 'bob.json'); + const ownerInbox = JSON.parse(fs.readFileSync(ownerInboxPath, 'utf8')); + expect(ownerInbox.at(-1).leadSessionId).toBe('session-review-2'); const kanbanCleared = parseJsonToolResult( await getTool('kanban_clear').execute({ diff --git a/src/main/services/team/TeamDataService.ts b/src/main/services/team/TeamDataService.ts index 8aa694c7..fa6ae4c9 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -1096,6 +1096,20 @@ export class TeamDataService { } } + private async resolveLeadRuntimeContext( + teamName: string + ): Promise<{ leadName: string; leadSessionId?: string }> { + try { + const config = await this.configReader.getConfig(teamName); + return { + leadName: this.resolveLeadNameFromConfig(config), + leadSessionId: config?.leadSessionId, + }; + } catch { + return { leadName: 'team-lead' }; + } + } + private isLeadOwner(owner: string, leadName: string): boolean { const normalized = owner.trim().toLowerCase(); if (!normalized) return false; @@ -1161,8 +1175,11 @@ export class TeamDataService { } async requestReview(teamName: string, taskId: string): Promise { - const leadName = await this.resolveLeadName(teamName); - this.getController(teamName).review.requestReview(taskId, { from: leadName }); + const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + this.getController(teamName).review.requestReview(taskId, { + from: leadName, + ...(leadSessionId ? { leadSessionId } : {}), + }); } async createTeamConfig(request: TeamCreateConfigRequest): Promise { @@ -1391,23 +1408,28 @@ export class TeamDataService { if (patch.op === 'set_column') { if (patch.column === 'review') { - const leadName = await this.resolveLeadName(teamName); - controller.review.requestReview(taskId, { from: leadName }); + const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + controller.review.requestReview(taskId, { + from: leadName, + ...(leadSessionId ? { leadSessionId } : {}), + }); } else { - const leadName = await this.resolveLeadName(teamName); + const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); controller.review.approveReview(taskId, { from: leadName, note: 'Approved from kanban', 'notify-owner': true, + ...(leadSessionId ? { leadSessionId } : {}), }); } return; } - const leadName = await this.resolveLeadName(teamName); + const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); controller.review.requestChanges(taskId, { from: leadName, comment: patch.comment?.trim() || 'Reviewer requested changes.', + ...(leadSessionId ? { leadSessionId } : {}), }); } diff --git a/src/main/services/team/TeamInboxReader.ts b/src/main/services/team/TeamInboxReader.ts index 0504e5d9..3e1473f8 100644 --- a/src/main/services/team/TeamInboxReader.ts +++ b/src/main/services/team/TeamInboxReader.ts @@ -86,7 +86,9 @@ export class TeamInboxReader { if ( typeof row.from !== 'string' || typeof row.text !== 'string' || - typeof row.timestamp !== 'string' + typeof row.timestamp !== 'string' || + typeof row.messageId !== 'string' || + row.messageId.trim().length === 0 ) { continue; } @@ -98,7 +100,7 @@ export class TeamInboxReader { read: typeof row.read === 'boolean' ? row.read : false, summary: typeof row.summary === 'string' ? row.summary : undefined, color: typeof row.color === 'string' ? row.color : undefined, - messageId: typeof row.messageId === 'string' ? row.messageId : undefined, + messageId: row.messageId, 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, diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 4fa391b8..781e2484 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -1095,7 +1095,6 @@ export class TeamProvisioningService { private readonly teamOpLocks = new Map>(); private readonly leadInboxRelayInFlight = new Map>(); private readonly relayedLeadInboxMessageIds = new Map>(); - private readonly relayedLeadInboxFallbackKeys = new Map>(); private readonly liveLeadProcessMessages = new Map(); private teamChangeEmitter: ((event: TeamChangeEvent) => void) | null = null; private helpOutputCache: string | null = null; @@ -2493,6 +2492,12 @@ export class TeamProvisioningService { * * Returns the number of messages relayed. */ + private hasStableMessageId( + message: InboxMessage + ): message is InboxMessage & { messageId: string } { + return typeof message.messageId === 'string' && message.messageId.trim().length > 0; + } + async relayLeadInboxMessages(teamName: string): Promise { const existing = this.leadInboxRelayInFlight.get(teamName); if (existing) { @@ -2507,7 +2512,6 @@ export class TeamProvisioningService { if (!run.provisioningComplete) return 0; const relayedIds = this.relayedLeadInboxMessageIds.get(teamName) ?? new Set(); - const relayedFallback = this.relayedLeadInboxFallbackKeys.get(teamName) ?? new Set(); let config: Awaited> | null = null; try { @@ -2528,13 +2532,11 @@ export class TeamProvisioningService { } const unread = leadInboxMessages - .filter((m) => { + .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 (typeof m.messageId === 'string' && m.messageId.trim().length > 0) { - return !relayedIds.has(m.messageId); - } - return !relayedFallback.has(`${m.timestamp}\0${m.from}\0${m.text}`); + if (!this.hasStableMessageId(m)) return false; + return !relayedIds.has(m.messageId); }) .sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)); @@ -2630,14 +2632,9 @@ export class TeamProvisioningService { } for (const m of batch) { - if (typeof m.messageId === 'string' && m.messageId.trim().length > 0) { - relayedIds.add(m.messageId); - } else { - relayedFallback.add(`${m.timestamp}\0${m.from}\0${m.text}`); - } + relayedIds.add(m.messageId); } this.relayedLeadInboxMessageIds.set(teamName, this.trimRelayedSet(relayedIds)); - this.relayedLeadInboxFallbackKeys.set(teamName, this.trimRelayedSet(relayedFallback)); try { await this.markInboxMessagesRead(teamName, leadName, batch); @@ -2786,7 +2783,7 @@ export class TeamProvisioningService { private async markInboxMessagesRead( teamName: string, member: string, - messages: { messageId?: string; timestamp: string; from: string; text: string }[] + messages: { messageId: string }[] ): Promise { const inboxPath = path.join(getTeamsBasePath(), teamName, 'inboxes', `${member}.json`); @@ -2807,27 +2804,14 @@ export class TeamProvisioningService { } if (!Array.isArray(parsed)) return; - const ids = new Set(messages.map((m) => m.messageId).filter((id): id is string => !!id)); - const fallbackKeys = new Set( - messages.filter((m) => !m.messageId).map((m) => `${m.timestamp}\0${m.from}\0${m.text}`) - ); + const ids = new Set(messages.map((m) => m.messageId).filter((id) => id.trim().length > 0)); let changed = false; for (const item of parsed) { if (!item || typeof item !== 'object') continue; const row = item as Record; const msgId = typeof row.messageId === 'string' ? row.messageId : null; - const timestamp = typeof row.timestamp === 'string' ? row.timestamp : null; - const from = typeof row.from === 'string' ? row.from : null; - const text = typeof row.text === 'string' ? row.text : null; - - const matchesId = msgId ? ids.has(msgId) : false; - const matchesFallback = - !msgId && timestamp && from && text - ? fallbackKeys.has(`${timestamp}\0${from}\0${text}`) - : false; - - if (!matchesId && !matchesFallback) continue; + if (!msgId || !ids.has(msgId)) continue; if (row.read !== true) { row.read = true; @@ -3873,7 +3857,6 @@ export class TeamProvisioningService { this.activeByTeam.delete(run.teamName); this.leadInboxRelayInFlight.delete(run.teamName); this.relayedLeadInboxMessageIds.delete(run.teamName); - this.relayedLeadInboxFallbackKeys.delete(run.teamName); this.liveLeadProcessMessages.delete(run.teamName); // Dismiss any pending tool approvals for this run if (run.pendingApprovals.size > 0) { diff --git a/src/main/services/team/TeamSentMessagesStore.ts b/src/main/services/team/TeamSentMessagesStore.ts index 6e7e5bd4..b225f903 100644 --- a/src/main/services/team/TeamSentMessagesStore.ts +++ b/src/main/services/team/TeamSentMessagesStore.ts @@ -59,7 +59,9 @@ export class TeamSentMessagesStore { if ( typeof row.from !== 'string' || typeof row.text !== 'string' || - typeof row.timestamp !== 'string' + typeof row.timestamp !== 'string' || + typeof row.messageId !== 'string' || + row.messageId.trim().length === 0 ) { continue; } @@ -71,7 +73,7 @@ export class TeamSentMessagesStore { timestamp: row.timestamp, read: typeof row.read === 'boolean' ? row.read : true, summary: typeof row.summary === 'string' ? row.summary : undefined, - messageId: typeof row.messageId === 'string' ? row.messageId : undefined, + messageId: row.messageId, color: typeof row.color === 'string' ? row.color : undefined, attachments: Array.isArray(row.attachments) ? row.attachments : undefined, source: typeof row.source === 'string' ? (row.source as InboxMessage['source']) : undefined, diff --git a/test/main/services/team/TeamDataService.test.ts b/test/main/services/team/TeamDataService.test.ts index 8d692c11..83100a3e 100644 --- a/test/main/services/team/TeamDataService.test.ts +++ b/test/main/services/team/TeamDataService.test.ts @@ -376,6 +376,7 @@ describe('TeamDataService', () => { getConfig: vi.fn(async () => ({ name: 'My team', members: [{ name: 'lead', role: 'team lead' }], + leadSessionId: 'lead-1', })), } as never, {} as never, @@ -397,6 +398,63 @@ describe('TeamDataService', () => { await service.requestReview('my-team', 'task-1'); - expect(requestReviewMock).toHaveBeenCalledWith('task-1', { from: 'lead' }); + expect(requestReviewMock).toHaveBeenCalledWith('task-1', { + from: 'lead', + leadSessionId: 'lead-1', + }); + }); + + it('propagates leadSessionId for kanban-driven review transitions', async () => { + const requestReviewMock = vi.fn(); + const approveReviewMock = vi.fn(); + const requestChangesMock = vi.fn(); + + const service = new TeamDataService( + { + listTeams: vi.fn(), + getConfig: vi.fn(async () => ({ + name: 'My team', + members: [{ name: 'lead', role: 'team lead' }], + leadSessionId: 'lead-2', + })), + } as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + () => + ({ + review: { + requestReview: requestReviewMock, + approveReview: approveReviewMock, + requestChanges: requestChangesMock, + }, + }) as never + ); + + await service.updateKanban('my-team', 'task-1', { op: 'set_column', column: 'review' }); + await service.updateKanban('my-team', 'task-1', { op: 'set_column', column: 'approved' }); + await service.updateKanban('my-team', 'task-1', { op: 'request_changes', comment: 'Needs fixes' }); + + expect(requestReviewMock).toHaveBeenCalledWith('task-1', { + from: 'lead', + leadSessionId: 'lead-2', + }); + expect(approveReviewMock).toHaveBeenCalledWith('task-1', { + from: 'lead', + note: 'Approved from kanban', + 'notify-owner': true, + leadSessionId: 'lead-2', + }); + expect(requestChangesMock).toHaveBeenCalledWith('task-1', { + from: 'lead', + comment: 'Needs fixes', + leadSessionId: 'lead-2', + }); }); }); diff --git a/test/main/services/team/TeamInboxReader.test.ts b/test/main/services/team/TeamInboxReader.test.ts index 545f6e5b..73539b91 100644 --- a/test/main/services/team/TeamInboxReader.test.ts +++ b/test/main/services/team/TeamInboxReader.test.ts @@ -96,6 +96,7 @@ describe('TeamInboxReader', () => { text: 'older', timestamp: '2026-01-01T00:00:00.000Z', read: false, + messageId: 'm-1', }, ]) ); @@ -107,6 +108,7 @@ describe('TeamInboxReader', () => { text: 'newer', timestamp: '2026-01-02T00:00:00.000Z', read: false, + messageId: 'm-2', }, ]) ); @@ -116,4 +118,30 @@ describe('TeamInboxReader', () => { expect(merged[0].text).toBe('newer'); expect(merged[1].text).toBe('older'); }); + + it('ignores legacy inbox rows without messageId', async () => { + hoisted.files.set( + '/mock/teams/my-team/inboxes/alice.json', + JSON.stringify([ + { + from: 'alice', + text: 'legacy', + timestamp: '2026-01-01T00:00:00.000Z', + read: false, + }, + { + from: 'alice', + text: 'supported', + timestamp: '2026-01-01T01:00:00.000Z', + read: false, + messageId: 'm-1', + }, + ]) + ); + + const messages = await reader.getMessagesFor('my-team', 'alice'); + expect(messages).toHaveLength(1); + expect(messages[0].text).toBe('supported'); + expect(messages[0].messageId).toBe('m-1'); + }); }); diff --git a/test/main/services/team/TeamProvisioningServiceRelay.test.ts b/test/main/services/team/TeamProvisioningServiceRelay.test.ts index 341a1727..76146444 100644 --- a/test/main/services/team/TeamProvisioningServiceRelay.test.ts +++ b/test/main/services/team/TeamProvisioningServiceRelay.test.ts @@ -272,4 +272,25 @@ describe('TeamProvisioningService relayLeadInboxMessages', () => { expect(second).toBe(1); expect(writeSpy).toHaveBeenCalledTimes(1); }); + + it('ignores unread lead inbox rows without messageId', async () => { + const service = new TeamProvisioningService(); + const teamName = 'my-team'; + seedConfig(teamName); + seedLeadInbox(teamName, [ + { + from: 'bob', + text: 'Legacy row without id', + timestamp: '2026-02-23T10:00:00.000Z', + read: false, + }, + ]); + + const { writeSpy } = attachAliveRun(service, teamName); + const relayed = await service.relayLeadInboxMessages(teamName); + + expect(relayed).toBe(0); + expect(writeSpy).toHaveBeenCalledTimes(0); + expect(hoisted.appendSentMessage).not.toHaveBeenCalled(); + }); });