From 86a1abdefa27da1950007a04c2ae753f09cda97a Mon Sep 17 00:00:00 2001 From: iliya Date: Mon, 9 Mar 2026 13:50:45 +0200 Subject: [PATCH] feat: introduce 'needsFix' review state and enhance task management features - Added 'needsFix' as a new review state to improve task tracking and management. - Updated kanban and task handling functions to accommodate the new review state, including modifications to clearKanban and task status updates. - Enhanced task briefing output to include sections for tasks needing fixes, pending tasks, and approved tasks, improving clarity in task management. - Updated UI components to display the 'needsFix' state appropriately across various views, including task rows and tooltips. - Refactored tests to cover new functionalities related to the 'needsFix' state, ensuring comprehensive coverage of task management scenarios. --- agent-teams-controller/src/internal/kanban.js | 4 +- .../src/internal/kanbanStore.js | 6 +- agent-teams-controller/src/internal/review.js | 16 ++--- .../src/internal/taskStore.js | 70 +++++++++++++++---- .../test/controller.test.js | 62 ++++++++++++++-- mcp-server/test/tools.test.ts | 24 ++++++- src/main/services/team/TeamDataService.ts | 20 +++--- .../services/team/TeamProvisioningService.ts | 17 +++-- src/main/workers/team-fs-worker.ts | 1 + .../sections/NotificationsSection.tsx | 3 +- .../components/sidebar/SidebarTaskItem.tsx | 9 ++- .../components/sidebar/taskFiltersState.ts | 21 ++++-- src/renderer/components/team/TaskTooltip.tsx | 9 ++- .../team/dialogs/TaskDetailDialog.tsx | 8 +++ .../components/team/kanban/KanbanTaskCard.tsx | 9 ++- .../team/members/MemberTasksTab.tsx | 8 +++ .../team/review/CodeMirrorDiffView.tsx | 26 ++++--- .../team/review/FileSectionDiff.tsx | 7 +- .../components/team/review/portionCollapse.ts | 8 +-- .../components/team/tasks/TaskRow.tsx | 23 ++++-- src/renderer/index.css | 46 ++++++++++-- src/renderer/store/slices/teamSlice.ts | 11 ++- src/renderer/utils/memberHelpers.ts | 10 +++ src/shared/types/team.ts | 2 +- src/shared/utils/reviewState.ts | 5 +- .../services/team/TeamDataService.test.ts | 8 +-- .../TeamProvisioningServicePrompts.test.ts | 9 ++- .../sidebar/taskFiltersState.test.ts | 17 +++++ test/shared/utils/reviewState.test.ts | 20 ++++++ 29 files changed, 384 insertions(+), 95 deletions(-) create mode 100644 test/renderer/components/sidebar/taskFiltersState.test.ts create mode 100644 test/shared/utils/reviewState.test.ts diff --git a/agent-teams-controller/src/internal/kanban.js b/agent-teams-controller/src/internal/kanban.js index c38744d8..fcfd731b 100644 --- a/agent-teams-controller/src/internal/kanban.js +++ b/agent-teams-controller/src/internal/kanban.js @@ -11,9 +11,9 @@ function setKanbanColumn(context, taskId, column) { return getKanbanState(context); } -function clearKanban(context, taskId) { +function clearKanban(context, taskId, options) { const canonicalTaskId = tasks.resolveTaskId(context, taskId); - kanbanStore.clearKanban(context.paths, context.teamName, canonicalTaskId); + kanbanStore.clearKanban(context.paths, context.teamName, canonicalTaskId, options); return getKanbanState(context); } diff --git a/agent-teams-controller/src/internal/kanbanStore.js b/agent-teams-controller/src/internal/kanbanStore.js index f19c4ee9..2ae590ae 100644 --- a/agent-teams-controller/src/internal/kanbanStore.js +++ b/agent-teams-controller/src/internal/kanbanStore.js @@ -88,13 +88,15 @@ function setKanbanColumn(paths, teamName, taskId, column) { return state; } -function clearKanban(paths, teamName, taskId) { +function clearKanban(paths, teamName, taskId, options = {}) { const state = readKanbanState(paths, teamName); delete state.tasks[String(taskId)]; writeKanbanState(paths, teamName, state); + const nextReviewState = + typeof options.nextReviewState === 'string' ? options.nextReviewState : 'none'; taskStore.updateTask(paths, String(taskId), (task) => ({ ...task, - reviewState: 'none', + reviewState: nextReviewState, })); return state; } diff --git a/agent-teams-controller/src/internal/review.js b/agent-teams-controller/src/internal/review.js index a42dc2f2..0ea22fd6 100644 --- a/agent-teams-controller/src/internal/review.js +++ b/agent-teams-controller/src/internal/review.js @@ -95,9 +95,9 @@ function approveReview(context, taskId, flags = {}) { from, text: note && note !== 'Approved' - ? `Task ${task.displayId || task.id} approved.\n\n${note}` - : `Task ${task.displayId || task.id} approved.`, - summary: `Approved ${task.displayId || task.id}`, + ? `Task #${task.displayId || task.id} approved.\n\n${note}` + : `Task #${task.displayId || task.id} approved.`, + summary: `Approved #${task.displayId || task.id}`, source: 'system_notification', ...(leadSessionId ? { leadSessionId } : {}), }); @@ -120,8 +120,8 @@ function requestChanges(context, taskId, flags = {}) { : 'Reviewer requested changes.'; const leadSessionId = resolveLeadSessionId(context, flags); - kanban.clearKanban(context, task.id); - tasks.setTaskStatus(context, task.id, 'in_progress', from); + kanban.clearKanban(context, task.id, { nextReviewState: 'needsFix' }); + tasks.setTaskStatus(context, task.id, 'pending', from); tasks.addTaskComment(context, task.id, { text: comment, from, @@ -131,9 +131,9 @@ function requestChanges(context, taskId, flags = {}) { to: task.owner, from, text: - `Task ${task.displayId || task.id} needs fixes.\n\n${comment}\n\n` + - 'Please fix and mark it as completed when ready.', - summary: `Fix request for ${task.displayId || task.id}`, + `Task #${task.displayId || task.id} needs fixes.\n\n${comment}\n\n` + + 'The task has been moved back to pending. When you are ready to resume, review the task context, start it explicitly, implement the fixes, mark it completed, and request review again.', + summary: `Fix request for #${task.displayId || task.id}`, source: 'system_notification', ...(leadSessionId ? { leadSessionId } : {}), }); diff --git a/agent-teams-controller/src/internal/taskStore.js b/agent-teams-controller/src/internal/taskStore.js index ed53193f..7e437223 100644 --- a/agent-teams-controller/src/internal/taskStore.js +++ b/agent-teams-controller/src/internal/taskStore.js @@ -3,7 +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 REVIEW_STATES = new Set(['none', 'review', 'needsFix', '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; @@ -578,15 +578,17 @@ function buildTaskReference(task) { return `#${task.displayId || deriveDisplayId(task.id)} (taskId: ${task.id})`; } -function compareTasksForBriefing(a, b) { - const order = { - in_progress: 0, - pending: 1, - completed: 2, - deleted: 3, - }; - const byStatus = (order[a.status] ?? 99) - (order[b.status] ?? 99); - if (byStatus !== 0) return byStatus; +function getTaskFreshness(task) { + const updated = Date.parse(String(task.updatedAt || '')); + if (Number.isFinite(updated) && updated > 0) return updated; + const created = Date.parse(String(task.createdAt || '')); + if (Number.isFinite(created) && created > 0) return created; + return 0; +} + +function compareTasksByFreshness(a, b) { + const freshnessDiff = getTaskFreshness(b) - getTaskFreshness(a); + if (freshnessDiff !== 0) return freshnessDiff; const byDisplay = String(a.displayId || a.id).localeCompare(String(b.displayId || b.id), undefined, { numeric: true, sensitivity: 'base', @@ -625,7 +627,7 @@ function formatTaskBriefing(paths, teamName, memberName) { }); const activeTasks = listTasks(paths) .filter((task) => task.owner === memberName && task.status !== 'deleted') - .sort(compareTasksForBriefing); + .sort(compareTasksByFreshness); if (activeTasks.length === 0) { return `No assigned tasks for ${memberName}.`; @@ -633,8 +635,26 @@ function formatTaskBriefing(paths, teamName, memberName) { const groups = { in_progress: activeTasks.filter((task) => task.status === 'in_progress'), - pending: activeTasks.filter((task) => task.status === 'pending'), - completed: activeTasks.filter((task) => task.status === 'completed'), + needs_fix: activeTasks.filter((task) => { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + return task.status !== 'in_progress' && getEffectiveReviewState(kanbanEntry, task) === 'needsFix'; + }), + pending: activeTasks.filter((task) => { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + return task.status === 'pending' && getEffectiveReviewState(kanbanEntry, task) === 'none'; + }), + review: activeTasks.filter((task) => { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + return getEffectiveReviewState(kanbanEntry, task) === 'review'; + }), + completed: activeTasks.filter((task) => { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + return task.status === 'completed' && getEffectiveReviewState(kanbanEntry, task) === 'none'; + }), + approved: activeTasks.filter((task) => { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + return getEffectiveReviewState(kanbanEntry, task) === 'approved'; + }), }; const lines = [`Task briefing for ${memberName}:`]; @@ -657,6 +677,14 @@ function formatTaskBriefing(paths, teamName, memberName) { } } + if (groups.needs_fix.length > 0) { + lines.push('', 'Needs fixes after review:'); + for (const task of groups.needs_fix) { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + lines.push(formatBriefTaskLine(task, getEffectiveReviewState(kanbanEntry, task))); + } + } + if (groups.pending.length > 0) { lines.push('', 'Pending:'); for (const task of groups.pending) { @@ -665,6 +693,14 @@ function formatTaskBriefing(paths, teamName, memberName) { } } + if (groups.review.length > 0) { + lines.push('', 'Review:'); + for (const task of groups.review) { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + lines.push(formatBriefTaskLine(task, getEffectiveReviewState(kanbanEntry, task))); + } + } + if (groups.completed.length > 0) { lines.push('', 'Completed:'); for (const task of groups.completed) { @@ -673,6 +709,14 @@ function formatTaskBriefing(paths, teamName, memberName) { } } + if (groups.approved.length > 0) { + lines.push('', 'Approved (last 10):'); + for (const task of groups.approved.slice(0, 10)) { + const kanbanEntry = kanbanState.tasks ? kanbanState.tasks[task.id] : undefined; + lines.push(formatBriefTaskLine(task, getEffectiveReviewState(kanbanEntry, task))); + } + } + return lines.join('\n'); } diff --git a/agent-teams-controller/test/controller.test.js b/agent-teams-controller/test/controller.test.js index 0159ee79..d6e5b1b4 100644 --- a/agent-teams-controller/test/controller.test.js +++ b/agent-teams-controller/test/controller.test.js @@ -118,7 +118,7 @@ describe('agent-teams-controller API', () => { expect(rows[1].id).toBe(registered.id); }); - it('keeps assigned tasks pending by default, supports explicit immediate start, notifies owners, and keeps briefing compact', async () => { + it('keeps assigned tasks pending by default, supports explicit immediate start, notifies owners, and groups briefing by review-aware sections', async () => { const claudeDir = makeClaudeDir(); const controller = createController({ teamName: 'my-team', claudeDir }); @@ -141,6 +141,30 @@ describe('agent-teams-controller API', () => { }); controller.tasks.completeTask(completedTask.id, 'bob'); controller.tasks.addTaskComment(activeTask.id, { from: 'bob', text: 'Resumed work with latest context.' }); + const needsFixTask = controller.tasks.createTask({ + subject: 'Fix after review', + owner: 'bob', + status: 'pending', + reviewState: 'needsFix', + createdAt: '2026-01-02T00:00:00.000Z', + notifyOwner: false, + }); + const reviewTask = controller.tasks.createTask({ + subject: 'Waiting for review', + owner: 'bob', + status: 'completed', + reviewState: 'review', + createdAt: '2026-01-03T00:00:00.000Z', + notifyOwner: false, + }); + const approvedTask = controller.tasks.createTask({ + subject: 'Approved work', + owner: 'bob', + status: 'completed', + reviewState: 'approved', + createdAt: '2026-01-04T00:00:00.000Z', + notifyOwner: false, + }); const reassignedTask = controller.tasks.createTask({ subject: 'Reassigned later' }); controller.tasks.setTaskOwner(reassignedTask.id, 'bob'); @@ -162,14 +186,20 @@ describe('agent-teams-controller API', () => { expect(briefing).toContain(`#${activeTask.displayId}`); expect(briefing).toContain('Description: Resume immediately'); expect(briefing).toContain('Resumed work with latest context.'); + expect(briefing).toContain('Needs fixes after review:'); + expect(briefing).toContain(`#${needsFixTask.displayId}`); expect(briefing).toContain('Pending:'); expect(briefing).toContain(`#${pendingTask.displayId}`); expect(briefing).not.toContain('Description: Do this later'); + expect(briefing).toContain('Review:'); + expect(briefing).toContain(`#${reviewTask.displayId}`); expect(briefing).toContain('Completed:'); expect(briefing).toContain(`#${completedTask.displayId}`); expect(briefing).not.toContain( 'Completed task description should stay out of compact rows' ); + expect(briefing).toContain('Approved (last 10):'); + expect(briefing).toContain(`#${approvedTask.displayId}`); }); it('reconciles stale kanban rows and linked inbox comments idempotently', () => { @@ -338,7 +368,7 @@ describe('agent-teams-controller API', () => { expect(rows[0].attachments[0].filename).toBe('note.txt'); }); - it('moves review back to in_progress and notifies owner on requestChanges', () => { + it('moves review back to pending+needsFix and notifies owner on requestChanges', () => { const claudeDir = makeClaudeDir(); const controller = createController({ teamName: 'my-team', claudeDir }); const task = controller.tasks.createTask({ subject: 'Needs revision', owner: 'bob' }); @@ -350,17 +380,41 @@ describe('agent-teams-controller API', () => { comment: 'Please address review feedback.', }); - expect(updated.status).toBe('in_progress'); - expect(updated.reviewState).toBe('none'); + expect(updated.status).toBe('pending'); + expect(updated.reviewState).toBe('needsFix'); expect(updated.comments.at(-1).type).toBe('review_request'); const inboxPath = path.join(claudeDir, 'teams', 'my-team', 'inboxes', 'bob.json'); 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).text).toContain('moved back to pending'); + expect(rows.at(-1).text).toContain('request review again'); expect(rows.at(-1).leadSessionId).toBe('lead-session-1'); }); + it('limits approved briefing section to the latest 10 tasks by freshness', async () => { + const claudeDir = makeClaudeDir(); + const controller = createController({ teamName: 'my-team', claudeDir }); + + const approvedTasks = Array.from({ length: 12 }, (_, index) => + controller.tasks.createTask({ + subject: `Approved ${index + 1}`, + owner: 'bob', + status: 'completed', + reviewState: 'approved', + createdAt: `2026-01-${String(index + 1).padStart(2, '0')}T00:00:00.000Z`, + }) + ); + + const briefing = await controller.tasks.taskBriefing('bob'); + expect(briefing).toContain('Approved (last 10):'); + expect(briefing).toContain(`#${approvedTasks[11].displayId}`); + expect(briefing).toContain(`#${approvedTasks[2].displayId}`); + expect(briefing).not.toContain(`#${approvedTasks[1].displayId}`); + expect(briefing).not.toContain(`#${approvedTasks[0].displayId}`); + }); + it('marks stale processes stopped during listing and supports unregister', () => { 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 ce309eb5..10d19dcd 100644 --- a/mcp-server/test/tools.test.ts +++ b/mcp-server/test/tools.test.ts @@ -419,11 +419,31 @@ describe('agent-teams-mcp tools', () => { }) ); - expect(changesRequested.status).toBe('in_progress'); - expect(changesRequested.reviewState).toBe('none'); + expect(changesRequested.status).toBe('pending'); + expect(changesRequested.reviewState).toBe('needsFix'); 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'); + expect(ownerInbox.at(-1).text).toContain('moved back to pending'); + + const taskByHashRef = parseJsonToolResult( + await getTool('task_get').execute({ + claudeDir, + teamName, + taskId: `#${createdTask.displayId}`, + }) + ); + expect(taskByHashRef.reviewState).toBe('needsFix'); + + const listedTasks = parseJsonToolResult( + await getTool('task_list').execute({ + claudeDir, + teamName, + }) + ); + expect(listedTasks.find((task: { id: string }) => task.id === createdTask.id)?.reviewState).toBe( + 'needsFix' + ); 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 93b46c34..9d3b2b80 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -84,7 +84,7 @@ export class TeamDataService { private readonly taskReader: TeamTaskReader = new TeamTaskReader(), private readonly inboxReader: TeamInboxReader = new TeamInboxReader(), _inboxWriter: TeamInboxWriter = new TeamInboxWriter(), - private readonly taskWriter: TeamTaskWriter = new TeamTaskWriter(), + _taskWriter: TeamTaskWriter = new TeamTaskWriter(), private readonly memberResolver: TeamMemberResolver = new TeamMemberResolver(), private readonly kanbanManager: TeamKanbanManager = new TeamKanbanManager(), _legacyToolsInstaller: unknown = null, @@ -107,7 +107,7 @@ export class TeamDataService { private resolveTaskReviewState( task: Pick - ): 'none' | 'review' | 'approved' { + ): 'none' | 'review' | 'needsFix' | 'approved' { return normalizeReviewState(task.reviewState); } @@ -1127,9 +1127,9 @@ export class TeamDataService { } async requestReview(teamName: string, taskId: string): Promise { - const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + const { leadSessionId } = await this.resolveLeadRuntimeContext(teamName); this.getController(teamName).review.requestReview(taskId, { - from: leadName, + from: 'user', ...(leadSessionId ? { leadSessionId } : {}), }); } @@ -1360,15 +1360,15 @@ export class TeamDataService { if (patch.op === 'set_column') { if (patch.column === 'review') { - const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + const { leadSessionId } = await this.resolveLeadRuntimeContext(teamName); controller.review.requestReview(taskId, { - from: leadName, + from: 'user', ...(leadSessionId ? { leadSessionId } : {}), }); } else { - const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + const { leadSessionId } = await this.resolveLeadRuntimeContext(teamName); controller.review.approveReview(taskId, { - from: leadName, + from: 'user', note: 'Approved from kanban', 'notify-owner': true, ...(leadSessionId ? { leadSessionId } : {}), @@ -1377,9 +1377,9 @@ export class TeamDataService { return; } - const { leadName, leadSessionId } = await this.resolveLeadRuntimeContext(teamName); + const { leadSessionId } = await this.resolveLeadRuntimeContext(teamName); controller.review.requestChanges(taskId, { - from: leadName, + from: 'user', comment: patch.comment?.trim() || 'Reviewer requested changes.', ...(leadSessionId ? { leadSessionId } : {}), }); diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index e58b305f..d00e018c 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -352,7 +352,7 @@ function buildMemberSpawnPrompt( ${getAgentLanguageInstruction()} Introduce yourself briefly (name and role) and confirm you are ready. Then wait for task assignments. -When you later receive work or reconnect after a restart, use task_briefing as your compact queue view, then call task_get for the specific task you are about to resume or start. +When you later receive work or reconnect after a restart, use task_briefing as your compact queue view. Use task_get when you need the full task context before starting a pending/needsFix task or when the in_progress briefing details are not enough. ${buildTeammateAgentBlockReminder()} Include the following agent-only instructions verbatim in the prompt: @@ -408,9 +408,11 @@ function buildTaskStatusProtocol(teamName: string): string { When you complete a task that blocks others, mention this in your completion message so blocked teammates can proceed. 13. TASK QUEUE DISCIPLINE: - Use task_briefing as a compact queue view of your assigned tasks. - - task_briefing may include full description/comments only for in_progress tasks; pending/completed entries may be minimal on purpose. - - Before resuming any in_progress task, call task_get for that task to refresh full context. - - When you become free, call task_briefing again, inspect pending tasks, then call task_get for the specific pending task you are about to start before task_start. + - task_briefing may include full description/comments only for in_progress tasks; needsFix/pending/review/completed entries may be minimal on purpose. + - Finish existing in_progress tasks first. + - If you need more context for an in_progress task, you MAY call task_get, but it is not mandatory when task_briefing already gives enough detail. + - Before starting a needsFix or pending task, call task_get for that specific task first, then run task_start only when you truly begin. + - If you complete fixes for a needsFix task, mark it completed and then send it back through review_request when ready for another review pass. Failure to follow this protocol means the task board will show incorrect status.`); } @@ -810,14 +812,15 @@ function buildLaunchPrompt( ${languageInstruction} The team has been reconnected after a restart. - ${hasTasks ? `You may have in_progress or pending tasks from the previous session.` : 'You have no assigned tasks currently.'} + ${hasTasks ? `You may have assigned tasks in states like in_progress, needsFix, pending, review, completed, or approved from the previous session.` : 'You have no assigned tasks currently.'} ${buildTeammateAgentBlockReminder()} Your FIRST action: call MCP tool task_briefing with: { teamName: "${request.teamName}", memberName: "${m.name}" } Then: - - If task_briefing shows any in_progress task, call task_get for each in_progress task first and resume/finish those before touching pending tasks. - - Only after there are no active in_progress tasks, inspect pending tasks from task_briefing, call task_get for the specific pending task you choose, and only then run task_start when you truly begin. + - If task_briefing shows any in_progress task, resume/finish those first. Call task_get only if you need more context than task_briefing already gave you. + - After that, prioritize tasks marked Needs fixes after review, then normal pending tasks. + - Before you start any needsFix or pending task, call task_get for that specific task, and only then run task_start when you truly begin. - If you have no tasks, wait for new assignments.`; }) .join('\n\n'); diff --git a/src/main/workers/team-fs-worker.ts b/src/main/workers/team-fs-worker.ts index 2e43f193..bd2e16a7 100644 --- a/src/main/workers/team-fs-worker.ts +++ b/src/main/workers/team-fs-worker.ts @@ -603,6 +603,7 @@ async function readTasksDirForTeam( : undefined; const reviewState = parsed.reviewState === 'review' || + parsed.reviewState === 'needsFix' || parsed.reviewState === 'approved' || parsed.reviewState === 'none' ? parsed.reviewState diff --git a/src/renderer/components/settings/sections/NotificationsSection.tsx b/src/renderer/components/settings/sections/NotificationsSection.tsx index 0ca33a92..330fd53b 100644 --- a/src/renderer/components/settings/sections/NotificationsSection.tsx +++ b/src/renderer/components/settings/sections/NotificationsSection.tsx @@ -17,7 +17,7 @@ import type { NotificationTrigger } from '@renderer/types/data'; import type { TeamReviewState, TeamTaskStatus } from '@shared/types'; /** Notification targets span workflow status plus the explicit review axis. */ -type NotifiableStatus = TeamTaskStatus | Extract; +type NotifiableStatus = TeamTaskStatus | Extract; // Snooze duration options const SNOOZE_OPTIONS = [ @@ -310,6 +310,7 @@ export const NotificationsSection = ({ const STATUS_OPTIONS: { value: NotifiableStatus; label: string }[] = [ { value: 'in_progress', label: 'Started' }, { value: 'completed', label: 'Completed' }, + { value: 'needsFix', label: 'Needs Fixes' }, { value: 'approved', label: 'Approved' }, { value: 'pending', label: 'Pending' }, { value: 'deleted', label: 'Deleted' }, diff --git a/src/renderer/components/sidebar/SidebarTaskItem.tsx b/src/renderer/components/sidebar/SidebarTaskItem.tsx index 15c7a711..dc017ec9 100644 --- a/src/renderer/components/sidebar/SidebarTaskItem.tsx +++ b/src/renderer/components/sidebar/SidebarTaskItem.tsx @@ -5,7 +5,7 @@ import { getTeamColorSet } from '@renderer/constants/teamColors'; import { useTheme } from '@renderer/hooks/useTheme'; import { useUnreadCommentCount } from '@renderer/hooks/useUnreadCommentCount'; import { useStore } from '@renderer/store'; -import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { REVIEW_STATE_DISPLAY, buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { nameColorSet } from '@renderer/utils/projectColor'; import { projectColor } from '@renderer/utils/projectColor'; import { projectLabelFromPath } from '@renderer/utils/taskGrouping'; @@ -205,6 +205,13 @@ export const SidebarTaskItem = ({ )} + {task.reviewState === 'needsFix' && !isRenaming ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} {unreadCount > 0 && !isRenaming && ( ({ }); export function taskMatchesStatus( - task: { status: string; reviewState?: 'none' | 'review' | 'approved'; kanbanColumn?: 'review' | 'approved' }, + task: { + status: string; + reviewState?: 'none' | 'review' | 'needsFix' | 'approved'; + kanbanColumn?: 'review' | 'approved'; + }, statusIds: Set ): boolean { if (statusIds.size === 0) return false; if (statusIds.size === STATUS_OPTIONS.length) return task.status !== 'deleted'; const kanbanColumn = getTaskKanbanColumn(task); - const inTodo = task.status === 'pending' && !kanbanColumn; + const inNeedsFix = task.reviewState === 'needsFix'; + const inTodo = task.status === 'pending' && !kanbanColumn && !inNeedsFix; const inProgress = task.status === 'in_progress' && !kanbanColumn; - const inDone = task.status === 'completed' && !kanbanColumn; + const inDone = task.status === 'completed' && !kanbanColumn && !inNeedsFix; const inReview = kanbanColumn === 'review'; const inApproved = kanbanColumn === 'approved'; return ( (statusIds.has('todo') && inTodo) || (statusIds.has('in_progress') && inProgress) || + (statusIds.has('needs_fix') && inNeedsFix) || (statusIds.has('done') && inDone) || (statusIds.has('review') && inReview) || (statusIds.has('approved') && inApproved) diff --git a/src/renderer/components/team/TaskTooltip.tsx b/src/renderer/components/team/TaskTooltip.tsx index cba866f1..b8dd6466 100644 --- a/src/renderer/components/team/TaskTooltip.tsx +++ b/src/renderer/components/team/TaskTooltip.tsx @@ -4,7 +4,7 @@ import { MarkdownViewer } from '@renderer/components/chat/viewers/MarkdownViewer 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 { REVIEW_STATE_DISPLAY, buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { getTaskKanbanColumn } from '@shared/utils/reviewState'; import { formatTaskDisplayLabel, taskMatchesRef } from '@shared/utils/taskIdentity'; @@ -100,6 +100,13 @@ export const TaskTooltip = ({ > {label} + {task.reviewState === 'needsFix' ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} {/* Owner */} {task.owner ? ( diff --git a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx index 5b7446f4..97af61b2 100644 --- a/src/renderer/components/team/dialogs/TaskDetailDialog.tsx +++ b/src/renderer/components/team/dialogs/TaskDetailDialog.tsx @@ -26,6 +26,7 @@ import { isImageMimeType } from '@renderer/utils/attachmentUtils'; import { buildMemberColorMap, KANBAN_COLUMN_DISPLAY, + REVIEW_STATE_DISPLAY, TASK_STATUS_LABELS, TASK_STATUS_STYLES, } from '@renderer/utils/memberHelpers'; @@ -339,6 +340,13 @@ export const TaskDetailDialog = ({ > {statusLabel} + {currentTask.reviewState === 'needsFix' ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} {headerExtra ?
{headerExtra}
: null} {editingSubject ? ( diff --git a/src/renderer/components/team/kanban/KanbanTaskCard.tsx b/src/renderer/components/team/kanban/KanbanTaskCard.tsx index 08b84b51..3f6d58e5 100644 --- a/src/renderer/components/team/kanban/KanbanTaskCard.tsx +++ b/src/renderer/components/team/kanban/KanbanTaskCard.tsx @@ -7,7 +7,7 @@ import { Popover, PopoverContent, PopoverTrigger } from '@renderer/components/ui import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; import { useUnreadCommentCount } from '@renderer/hooks/useUnreadCommentCount'; import { useStore } from '@renderer/store'; -import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { REVIEW_STATE_DISPLAY, buildMemberColorMap } from '@renderer/utils/memberHelpers'; import { deriveTaskDisplayId, formatTaskDisplayLabel } from '@shared/utils/taskIdentity'; import { ArrowLeftFromLine, @@ -288,6 +288,13 @@ export const KanbanTaskCard = ({ {task.needsClarification === 'user' ? 'Awaiting user' : 'Awaiting lead'} ) : null} + {task.reviewState === 'needsFix' ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} {compact && } diff --git a/src/renderer/components/team/members/MemberTasksTab.tsx b/src/renderer/components/team/members/MemberTasksTab.tsx index 7c67e028..f3150d81 100644 --- a/src/renderer/components/team/members/MemberTasksTab.tsx +++ b/src/renderer/components/team/members/MemberTasksTab.tsx @@ -3,6 +3,7 @@ import { useMemo } from 'react'; import { Badge } from '@renderer/components/ui/badge'; import { KANBAN_COLUMN_DISPLAY, + REVIEW_STATE_DISPLAY, TASK_STATUS_LABELS, TASK_STATUS_STYLES, } from '@renderer/utils/memberHelpers'; @@ -70,6 +71,13 @@ export const MemberTasksTab = ({ tasks, onTaskClick }: MemberTasksTabProps): Rea > {label} + {task.reviewState === 'needsFix' ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} ); })} diff --git a/src/renderer/components/team/review/CodeMirrorDiffView.tsx b/src/renderer/components/team/review/CodeMirrorDiffView.tsx index e1d532e7..781bd2b5 100644 --- a/src/renderer/components/team/review/CodeMirrorDiffView.tsx +++ b/src/renderer/components/team/review/CodeMirrorDiffView.tsx @@ -82,10 +82,14 @@ function computeHunkIndexAtPos(state: EditorState, pos: number): number { /** Diff-specific theme — merge toolbar, changed/deleted line backgrounds, collapse markers */ const diffSpecificTheme = EditorView.theme({ - '.cm-changedLine': { backgroundColor: '#1a3a1a !important' }, - '.cm-deletedChunk': { backgroundColor: '#241517', position: 'relative', overflow: 'visible' }, - '.cm-insertedLine': { backgroundColor: '#1a3a1a !important' }, - '.cm-deletedLine': { backgroundColor: '#241517 !important' }, + '.cm-changedLine': { backgroundColor: 'var(--diff-cm-changed-bg) !important' }, + '.cm-deletedChunk': { + backgroundColor: 'var(--diff-cm-deleted-bg)', + position: 'relative', + overflow: 'visible', + }, + '.cm-insertedLine': { backgroundColor: 'var(--diff-cm-changed-bg) !important' }, + '.cm-deletedLine': { backgroundColor: 'var(--diff-cm-deleted-bg) !important' }, // Merge toolbar — absolute, Y set dynamically by mousemove handler '.cm-deletedChunk .cm-chunkButtons': { position: 'absolute', @@ -121,7 +125,7 @@ const diffSpecificTheme = EditorView.theme({ padding: '3px 8px', fontSize: '13px', lineHeight: '20px', - '&:hover': { background: 'rgba(255,255,255,0.08)' }, + '&:hover': { background: 'var(--diff-merge-nav-hover-bg)' }, }, '.cm-merge-nav-counter': { fontSize: '12px', @@ -139,7 +143,7 @@ const diffSpecificTheme = EditorView.theme({ color: 'var(--color-text)', backgroundColor: 'var(--color-surface-raised)', border: '1px solid var(--color-border)', - '&:hover': { backgroundColor: 'rgba(255,255,255,0.1)' }, + '&:hover': { backgroundColor: 'var(--diff-merge-undo-hover-bg)' }, '& kbd': { fontSize: '10px', color: 'var(--color-text-muted)', marginLeft: '4px' }, }, '.cm-merge-keep': { @@ -149,11 +153,11 @@ const diffSpecificTheme = EditorView.theme({ fontSize: '12px', fontWeight: '500', lineHeight: '20px', - color: '#3fb950', - backgroundColor: 'rgba(46, 160, 67, 0.25)', - border: '1px solid rgba(46, 160, 67, 0.4)', - '&:hover': { backgroundColor: 'rgba(46, 160, 67, 0.4)' }, - '& kbd': { fontSize: '10px', color: 'rgba(63, 185, 80, 0.7)', marginLeft: '4px' }, + color: 'var(--diff-merge-keep-color)', + backgroundColor: 'var(--diff-merge-keep-bg)', + border: '1px solid var(--diff-merge-keep-border)', + '&:hover': { backgroundColor: 'var(--diff-merge-keep-hover-bg)' }, + '& kbd': { fontSize: '10px', color: 'var(--diff-merge-keep-kbd)', marginLeft: '4px' }, }, // Collapse unchanged region marker '.cm-collapsedLines': { diff --git a/src/renderer/components/team/review/FileSectionDiff.tsx b/src/renderer/components/team/review/FileSectionDiff.tsx index a2ca0f63..324316ff 100644 --- a/src/renderer/components/team/review/FileSectionDiff.tsx +++ b/src/renderer/components/team/review/FileSectionDiff.tsx @@ -116,9 +116,12 @@ export const FileSectionDiff = ({ return (
{isMissingOnDisk && ( -
+
File is missing on disk. This diff may be only a preview from agent logs. Use{' '} - Restore to create the file on disk. + Restore to create the file on disk.
)} { {task.owner ?? 'Unassigned'} - {kanbanColumn && kanbanColumn in KANBAN_COLUMN_DISPLAY - ? KANBAN_COLUMN_DISPLAY[kanbanColumn].label - : (TASK_STATUS_LABELS[task.status] ?? task.status)} +
+ + {kanbanColumn && kanbanColumn in KANBAN_COLUMN_DISPLAY + ? KANBAN_COLUMN_DISPLAY[kanbanColumn].label + : (TASK_STATUS_LABELS[task.status] ?? task.status)} + + {task.reviewState === 'needsFix' ? ( + + {REVIEW_STATE_DISPLAY.needsFix.label} + + ) : null} +
{blockedByIds.length > 0 ? ( diff --git a/src/renderer/index.css b/src/renderer/index.css index c9bc3050..3275c12c 100644 --- a/src/renderer/index.css +++ b/src/renderer/index.css @@ -81,6 +81,23 @@ --diff-removed-bg: rgba(239, 68, 68, 0.15); --diff-removed-text: #f87171; --diff-removed-border: #ef4444; + /* CodeMirror diff backgrounds */ + --diff-cm-changed-bg: #1a3a1a; + --diff-cm-deleted-bg: #241517; + /* CodeMirror merge buttons */ + --diff-merge-undo-hover-bg: rgba(255, 255, 255, 0.1); + --diff-merge-keep-color: #3fb950; + --diff-merge-keep-bg: rgba(46, 160, 67, 0.25); + --diff-merge-keep-border: rgba(46, 160, 67, 0.4); + --diff-merge-keep-hover-bg: rgba(46, 160, 67, 0.4); + --diff-merge-keep-kbd: rgba(63, 185, 80, 0.7); + /* CodeMirror merge nav button hover */ + --diff-merge-nav-hover-bg: rgba(255, 255, 255, 0.08); + /* Portion collapse expand buttons */ + --diff-expand-hover-bg: rgba(255, 255, 255, 0.06); + --diff-expand-active-bg: rgba(255, 255, 255, 0.1); + --diff-expand-all-hover-bg: rgba(255, 255, 255, 0.04); + --diff-expand-all-active-bg: rgba(255, 255, 255, 0.08); /* Markdown prose */ --prose-heading: #f8fafc; @@ -308,12 +325,29 @@ --inline-code-text: #3a3935; /* Diff viewer - Light mode */ - --diff-added-bg: rgba(34, 197, 94, 0.1); - --diff-added-text: #166534; - --diff-added-border: #22c55e; - --diff-removed-bg: rgba(239, 68, 68, 0.1); - --diff-removed-text: #991b1b; - --diff-removed-border: #ef4444; + --diff-added-bg: rgba(34, 197, 94, 0.18); + --diff-added-text: #14532d; + --diff-added-border: #16a34a; + --diff-removed-bg: rgba(239, 68, 68, 0.18); + --diff-removed-text: #7f1d1d; + --diff-removed-border: #dc2626; + /* CodeMirror diff backgrounds - Light mode */ + --diff-cm-changed-bg: rgba(34, 197, 94, 0.14); + --diff-cm-deleted-bg: rgba(239, 68, 68, 0.14); + /* CodeMirror merge buttons - Light mode */ + --diff-merge-undo-hover-bg: rgba(0, 0, 0, 0.08); + --diff-merge-keep-color: #15803d; + --diff-merge-keep-bg: rgba(34, 197, 94, 0.15); + --diff-merge-keep-border: rgba(34, 197, 94, 0.4); + --diff-merge-keep-hover-bg: rgba(34, 197, 94, 0.25); + --diff-merge-keep-kbd: rgba(21, 128, 61, 0.6); + /* CodeMirror merge nav button hover - Light mode */ + --diff-merge-nav-hover-bg: rgba(0, 0, 0, 0.06); + /* Portion collapse expand buttons - Light mode */ + --diff-expand-hover-bg: rgba(0, 0, 0, 0.05); + --diff-expand-active-bg: rgba(0, 0, 0, 0.08); + --diff-expand-all-hover-bg: rgba(0, 0, 0, 0.03); + --diff-expand-all-active-bg: rgba(0, 0, 0, 0.06); /* Markdown prose - Warm tones */ --prose-heading: #1c1b19; diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index 681334e4..6c2389ea 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -161,9 +161,10 @@ function detectStatusChangeNotifications( const taskKanbanColumn = getTaskKanbanColumn(task); const oldTaskKanbanColumn = getTaskKanbanColumn(oldTask); const becameApproved = taskKanbanColumn === 'approved' && oldTaskKanbanColumn !== 'approved'; + const becameNeedsFix = task.reviewState === 'needsFix' && oldTask.reviewState !== 'needsFix'; const statusChanged = oldTask.status !== task.status; - if (!statusChanged && !becameApproved) continue; + if (!statusChanged && !becameApproved && !becameNeedsFix) continue; if (onlySolo) { const team = teamByName[task.teamName]; @@ -171,7 +172,7 @@ function detectStatusChangeNotifications( } // Resolve the effective status for notification matching - const effectiveStatus = becameApproved ? 'approved' : task.status; + const effectiveStatus = becameApproved ? 'approved' : becameNeedsFix ? 'needsFix' : task.status; if (!statuses.includes(effectiveStatus)) continue; const key = `${task.teamName}:${task.id}:${effectiveStatus}`; @@ -182,7 +183,7 @@ function detectStatusChangeNotifications( fireStatusChangeNotification( task, fromLabel, - becameApproved ? 'approved' : undefined, + becameApproved ? 'approved' : becameNeedsFix ? 'needsFix' : undefined, !statusChangeEnabled ); } @@ -199,6 +200,7 @@ function fireStatusChangeNotification( in_progress: 'In Progress', completed: 'Completed', deleted: 'Deleted', + needsFix: 'Needs Fixes', approved: 'Approved', }; const from = statusLabels[fromStatus] ?? fromStatus; @@ -552,6 +554,9 @@ export const createTeamSlice: StateCreator = (set, notifiedClarificationTaskKeys.add(`${task.teamName}:${task.id}`); } notifiedStatusChangeKeys.add(`${task.teamName}:${task.id}:${task.status}`); + if (task.reviewState === 'needsFix') { + notifiedStatusChangeKeys.add(`${task.teamName}:${task.id}:needsFix`); + } if (getTaskKanbanColumn(task) === 'approved') { notifiedStatusChangeKeys.add(`${task.teamName}:${task.id}:approved`); } diff --git a/src/renderer/utils/memberHelpers.ts b/src/renderer/utils/memberHelpers.ts index 2c75d370..8fb8c53b 100644 --- a/src/renderer/utils/memberHelpers.ts +++ b/src/renderer/utils/memberHelpers.ts @@ -4,6 +4,7 @@ import type { LeadActivityState, MemberStatus, ResolvedTeamMember, + TeamReviewState, TeamTaskStatus, } from '@shared/types'; @@ -131,3 +132,12 @@ export const KANBAN_COLUMN_DISPLAY: Record< review: { label: 'In Review', bg: 'bg-amber-500/15', text: 'text-amber-400' }, approved: { label: 'Approved', bg: 'bg-emerald-500/15', text: 'text-emerald-400' }, }; + +export const REVIEW_STATE_DISPLAY: Record< + Exclude, + { label: string; bg: string; text: string } +> = { + review: { label: 'In Review', bg: 'bg-amber-500/15', text: 'text-amber-400' }, + needsFix: { label: 'Needs Fixes', bg: 'bg-rose-500/15', text: 'text-rose-400' }, + approved: { label: 'Approved', bg: 'bg-emerald-500/15', text: 'text-emerald-400' }, +}; diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index 614b51de..45e6d9e3 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -56,7 +56,7 @@ export interface TeamSummary { } export type TeamTaskStatus = 'pending' | 'in_progress' | 'completed' | 'deleted'; -export type TeamReviewState = 'none' | 'review' | 'approved'; +export type TeamReviewState = 'none' | 'review' | 'needsFix' | 'approved'; export interface TaskWorkInterval { /** ISO timestamp when task entered in_progress */ diff --git a/src/shared/utils/reviewState.ts b/src/shared/utils/reviewState.ts index ae649bde..5ed3241f 100644 --- a/src/shared/utils/reviewState.ts +++ b/src/shared/utils/reviewState.ts @@ -7,7 +7,7 @@ interface ReviewStateLike { } export function normalizeReviewState(value: unknown): TeamReviewState { - return value === 'review' || value === 'approved' ? value : 'none'; + return value === 'review' || value === 'needsFix' || value === 'approved' ? value : 'none'; } export function getReviewStateFromTask(task: ReviewStateLike): TeamReviewState { @@ -41,3 +41,6 @@ export function isReviewTask(task: ReviewStateLike): boolean { return getReviewStateFromTask(task) === 'review'; } +export function isNeedsFixTask(task: ReviewStateLike): boolean { + return getReviewStateFromTask(task) === 'needsFix'; +} diff --git a/test/main/services/team/TeamDataService.test.ts b/test/main/services/team/TeamDataService.test.ts index 3648d11c..c7ca10bd 100644 --- a/test/main/services/team/TeamDataService.test.ts +++ b/test/main/services/team/TeamDataService.test.ts @@ -457,7 +457,7 @@ describe('TeamDataService', () => { await service.requestReview('my-team', 'task-1'); expect(requestReviewMock).toHaveBeenCalledWith('task-1', { - from: 'lead', + from: 'user', leadSessionId: 'lead-1', }); }); @@ -500,17 +500,17 @@ describe('TeamDataService', () => { await service.updateKanban('my-team', 'task-1', { op: 'request_changes', comment: 'Needs fixes' }); expect(requestReviewMock).toHaveBeenCalledWith('task-1', { - from: 'lead', + from: 'user', leadSessionId: 'lead-2', }); expect(approveReviewMock).toHaveBeenCalledWith('task-1', { - from: 'lead', + from: 'user', note: 'Approved from kanban', 'notify-owner': true, leadSessionId: 'lead-2', }); expect(requestChangesMock).toHaveBeenCalledWith('task-1', { - from: 'lead', + from: 'user', comment: 'Needs fixes', leadSessionId: 'lead-2', }); diff --git a/test/main/services/team/TeamProvisioningServicePrompts.test.ts b/test/main/services/team/TeamProvisioningServicePrompts.test.ts index 729d736d..3a5223e7 100644 --- a/test/main/services/team/TeamProvisioningServicePrompts.test.ts +++ b/test/main/services/team/TeamProvisioningServicePrompts.test.ts @@ -217,8 +217,10 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(prompt).toContain(` ${AGENT_BLOCK_CLOSE}`); expect(prompt).toContain('NEVER use agent-only blocks in messages to "user".'); expect(prompt).toContain('use task_briefing as your compact queue view'); - expect(prompt).toContain('then call task_get for the specific task you are about to resume or start'); + 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.'); + expect(prompt).toContain('you MAY call task_get'); + expect(prompt).toContain('Before starting a needsFix or pending task, call task_get'); await svc.cancelProvisioning(runId); }); @@ -277,8 +279,9 @@ describe('TeamProvisioningService prompt content (solo mode discipline)', () => expect(prompt).toContain(` ${AGENT_BLOCK_CLOSE}`); expect(prompt).toContain('NEVER use agent-only blocks in messages to "user".'); expect(prompt).toContain('Your FIRST action: call MCP tool task_briefing'); - expect(prompt).toContain('call task_get for each in_progress task first'); - expect(prompt).toContain('call task_get for the specific pending task you choose'); + 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'); + expect(prompt).toContain('Before you start any needsFix or pending task, call task_get'); await svc.cancelProvisioning(runId); }); diff --git a/test/renderer/components/sidebar/taskFiltersState.test.ts b/test/renderer/components/sidebar/taskFiltersState.test.ts new file mode 100644 index 00000000..dcce0001 --- /dev/null +++ b/test/renderer/components/sidebar/taskFiltersState.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; + +import { taskMatchesStatus } from '../../../../src/renderer/components/sidebar/taskFiltersState'; + +describe('taskFiltersState', () => { + it('treats needsFix as distinct from normal todo/done buckets', () => { + const pendingNeedsFixTask = { status: 'pending', reviewState: 'needsFix' as const }; + const completedNeedsFixTask = { status: 'completed', reviewState: 'needsFix' as const }; + const normalPendingTask = { status: 'pending', reviewState: 'none' as const }; + + expect(taskMatchesStatus(pendingNeedsFixTask, new Set(['needs_fix']))).toBe(true); + expect(taskMatchesStatus(completedNeedsFixTask, new Set(['needs_fix']))).toBe(true); + expect(taskMatchesStatus(pendingNeedsFixTask, new Set(['todo']))).toBe(false); + expect(taskMatchesStatus(completedNeedsFixTask, new Set(['done']))).toBe(false); + expect(taskMatchesStatus(normalPendingTask, new Set(['todo']))).toBe(true); + }); +}); diff --git a/test/shared/utils/reviewState.test.ts b/test/shared/utils/reviewState.test.ts new file mode 100644 index 00000000..783a4637 --- /dev/null +++ b/test/shared/utils/reviewState.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest'; + +import { + getKanbanColumnFromReviewState, + getReviewStateFromTask, + isNeedsFixTask, + normalizeReviewState, +} from '../../../src/shared/utils/reviewState'; + +describe('reviewState utils', () => { + it('normalizes needsFix as a first-class review state', () => { + expect(normalizeReviewState('needsFix')).toBe('needsFix'); + expect(getReviewStateFromTask({ reviewState: 'needsFix' })).toBe('needsFix'); + expect(isNeedsFixTask({ reviewState: 'needsFix' })).toBe(true); + }); + + it('does not map needsFix to a kanban column', () => { + expect(getKanbanColumnFromReviewState('needsFix')).toBeUndefined(); + }); +});