From 416ef3a3940c7cdd4d8bcc4c9ad80e8313bd1b12 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 16 Feb 2026 18:58:42 -0800 Subject: [PATCH 1/3] feat: Add error handling to auto-mode facade and implement followUp feature. Fix Claude weekly usage indicator. Fix mobile card drag --- apps/server/src/services/auto-mode/compat.ts | 5 +- apps/server/src/services/auto-mode/facade.ts | 155 ++++++++++++++---- apps/server/src/services/auto-mode/index.ts | 1 + apps/server/src/services/auto-mode/types.ts | 20 +++ .../src/services/claude-usage-service.ts | 37 ++++- .../src/services/feature-state-manager.ts | 41 +++++ .../services/claude-usage-service.test.ts | 60 +++++++ .../services/feature-state-manager.test.ts | 90 ++++++++++ .../src/components/ui/task-progress-panel.tsx | 55 ++++--- apps/ui/src/components/usage-popover.tsx | 51 +++++- .../kanban-card/agent-info-panel.tsx | 12 ++ .../components/kanban-card/card-header.tsx | 9 +- .../components/kanban-card/kanban-card.tsx | 8 +- .../views/board-view/mobile-usage-bar.tsx | 25 ++- apps/ui/src/store/utils/usage-utils.ts | 59 +++++++ 15 files changed, 552 insertions(+), 76 deletions(-) diff --git a/apps/server/src/services/auto-mode/compat.ts b/apps/server/src/services/auto-mode/compat.ts index cece24750..2c713c011 100644 --- a/apps/server/src/services/auto-mode/compat.ts +++ b/apps/server/src/services/auto-mode/compat.ts @@ -13,6 +13,7 @@ import { GlobalAutoModeService } from './global-service.js'; import { AutoModeServiceFacade } from './facade.js'; import type { SettingsService } from '../settings-service.js'; import type { FeatureLoader } from '../feature-loader.js'; +import type { ClaudeUsageService } from '../claude-usage-service.js'; import type { FacadeOptions, AutoModeStatus, RunningAgentInfo } from './types.js'; /** @@ -27,7 +28,8 @@ export class AutoModeServiceCompat { constructor( events: EventEmitter, settingsService: SettingsService | null, - featureLoader: FeatureLoader + featureLoader: FeatureLoader, + claudeUsageService?: ClaudeUsageService | null ) { this.globalService = new GlobalAutoModeService(events, settingsService, featureLoader); const sharedServices = this.globalService.getSharedServices(); @@ -37,6 +39,7 @@ export class AutoModeServiceCompat { settingsService, featureLoader, sharedServices, + claudeUsageService: claudeUsageService ?? null, }; } diff --git a/apps/server/src/services/auto-mode/facade.ts b/apps/server/src/services/auto-mode/facade.ts index 5d8bde58b..e31543b41 100644 --- a/apps/server/src/services/auto-mode/facade.ts +++ b/apps/server/src/services/auto-mode/facade.ts @@ -38,6 +38,7 @@ import type { SettingsService } from '../settings-service.js'; import type { EventEmitter } from '../../lib/events.js'; import type { FacadeOptions, + FacadeError, AutoModeStatus, ProjectAutoModeStatus, WorktreeCapacityInfo, @@ -89,6 +90,45 @@ export class AutoModeServiceFacade { private readonly settingsService: SettingsService | null ) {} + /** + * Classify and log an error at the facade boundary. + * Emits an error event to the UI so failures are surfaced to the user. + * + * @param error - The caught error + * @param method - The facade method name where the error occurred + * @param featureId - Optional feature ID for context + * @returns The classified FacadeError for structured consumption + */ + private handleFacadeError(error: unknown, method: string, featureId?: string): FacadeError { + const errorInfo = classifyError(error); + + // Log at the facade boundary for debugging + logger.error( + `[${method}] ${featureId ? `Feature ${featureId}: ` : ''}${errorInfo.message}`, + error + ); + + // Emit error event to UI unless it's an abort/cancellation + if (!errorInfo.isAbort && !errorInfo.isCancellation) { + this.eventBus.emitAutoModeEvent('auto_mode_error', { + featureId: featureId ?? null, + featureName: undefined, + branchName: null, + error: errorInfo.message, + errorType: errorInfo.type, + projectPath: this.projectPath, + }); + } + + return { + method, + errorType: errorInfo.type, + message: errorInfo.message, + featureId, + projectPath: this.projectPath, + }; + } + /** * Create a new AutoModeServiceFacade instance for a specific project. * @@ -447,11 +487,16 @@ export class AutoModeServiceFacade { * @param maxConcurrency - Maximum concurrent features */ async startAutoLoop(branchName: string | null = null, maxConcurrency?: number): Promise { - return this.autoLoopCoordinator.startAutoLoopForProject( - this.projectPath, - branchName, - maxConcurrency - ); + try { + return await this.autoLoopCoordinator.startAutoLoopForProject( + this.projectPath, + branchName, + maxConcurrency + ); + } catch (error) { + this.handleFacadeError(error, 'startAutoLoop'); + throw error; + } } /** @@ -459,7 +504,12 @@ export class AutoModeServiceFacade { * @param branchName - The branch name, or null for main worktree */ async stopAutoLoop(branchName: string | null = null): Promise { - return this.autoLoopCoordinator.stopAutoLoopForProject(this.projectPath, branchName); + try { + return await this.autoLoopCoordinator.stopAutoLoopForProject(this.projectPath, branchName); + } catch (error) { + this.handleFacadeError(error, 'stopAutoLoop'); + throw error; + } } /** @@ -500,14 +550,19 @@ export class AutoModeServiceFacade { _calledInternally?: boolean; } ): Promise { - return this.executionService.executeFeature( - this.projectPath, - featureId, - useWorktrees, - isAutoMode, - providedWorktreePath, - options - ); + try { + return await this.executionService.executeFeature( + this.projectPath, + featureId, + useWorktrees, + isAutoMode, + providedWorktreePath, + options + ); + } catch (error) { + this.handleFacadeError(error, 'executeFeature', featureId); + throw error; + } } /** @@ -515,9 +570,14 @@ export class AutoModeServiceFacade { * @param featureId - ID of the feature to stop */ async stopFeature(featureId: string): Promise { - // Cancel any pending plan approval for this feature - this.cancelPlanApproval(featureId); - return this.executionService.stopFeature(featureId); + try { + // Cancel any pending plan approval for this feature + this.cancelPlanApproval(featureId); + return await this.executionService.stopFeature(featureId); + } catch (error) { + this.handleFacadeError(error, 'stopFeature', featureId); + throw error; + } } /** @@ -552,23 +612,54 @@ export class AutoModeServiceFacade { imagePaths?: string[], useWorktrees = true ): Promise { - // Stub: acquire concurrency slot then immediately throw. - // Heavy I/O (loadFeature, worktree resolution, context reading, prompt building) - // is deferred to the real AutoModeService.followUpFeature implementation. validateWorkingDirectory(this.projectPath); - const runningEntry = this.concurrencyManager.acquire({ - featureId, - projectPath: this.projectPath, - isAutoMode: false, - }); - try { - // NOTE: Facade does not have runAgent - this method requires AutoModeService - // Do NOT emit start events before throwing to prevent false start events - throw new Error( - 'followUpFeature not fully implemented in facade - use AutoModeService.followUpFeature instead' - ); + // Load feature to build the prompt context + const feature = await this.featureStateManager.loadFeature(this.projectPath, featureId); + if (!feature) throw new Error(`Feature ${featureId} not found`); + + // Read previous agent output as context + const featureDir = getFeatureDir(this.projectPath, featureId); + let previousContext = ''; + try { + previousContext = (await secureFs.readFile( + path.join(featureDir, 'agent-output.md'), + 'utf-8' + )) as string; + } catch { + // No previous context available - that's OK + } + + // Build the feature prompt section + const featurePrompt = `## Feature Implementation Task\n\n**Feature ID:** ${feature.id}\n**Title:** ${feature.title || 'Untitled Feature'}\n**Description:** ${feature.description}\n`; + + // Get the follow-up prompt template and build the continuation prompt + const prompts = await getPromptCustomization(this.settingsService, '[Facade]'); + let continuationPrompt = prompts.autoMode.followUpPromptTemplate; + continuationPrompt = continuationPrompt + .replace(/\{\{featurePrompt\}\}/g, featurePrompt) + .replace(/\{\{previousContext\}\}/g, previousContext) + .replace(/\{\{followUpInstructions\}\}/g, prompt); + + // Store image paths on the feature so executeFeature can pick them up + if (imagePaths && imagePaths.length > 0) { + feature.imagePaths = imagePaths.map((p) => ({ + path: p, + filename: p.split('/').pop() || p, + mimeType: 'image/*', + })); + await this.featureStateManager.updateFeatureStatus( + this.projectPath, + featureId, + feature.status || 'in_progress' + ); + } + + // Delegate to executeFeature with the built continuation prompt + await this.executeFeature(featureId, useWorktrees, false, undefined, { + continuationPrompt, + }); } catch (error) { const errorInfo = classifyError(error); if (!errorInfo.isAbort) { @@ -582,8 +673,6 @@ export class AutoModeServiceFacade { }); } throw error; - } finally { - this.concurrencyManager.release(featureId); } } diff --git a/apps/server/src/services/auto-mode/index.ts b/apps/server/src/services/auto-mode/index.ts index 9e150ad20..40e0ee848 100644 --- a/apps/server/src/services/auto-mode/index.ts +++ b/apps/server/src/services/auto-mode/index.ts @@ -58,6 +58,7 @@ export type { WorktreeCapacityInfo, RunningAgentInfo, OrphanedFeatureInfo, + FacadeError, GlobalAutoModeOperations, } from './types.js'; diff --git a/apps/server/src/services/auto-mode/types.ts b/apps/server/src/services/auto-mode/types.ts index b831dabaa..fc82cb139 100644 --- a/apps/server/src/services/auto-mode/types.ts +++ b/apps/server/src/services/auto-mode/types.ts @@ -15,6 +15,7 @@ import type { ConcurrencyManager } from '../concurrency-manager.js'; import type { AutoLoopCoordinator } from '../auto-loop-coordinator.js'; import type { WorktreeResolver } from '../worktree-resolver.js'; import type { TypedEventBus } from '../typed-event-bus.js'; +import type { ClaudeUsageService } from '../claude-usage-service.js'; // Re-export types from extracted services for route consumption export type { AutoModeConfig, ProjectAutoLoopState } from '../auto-loop-coordinator.js'; @@ -55,6 +56,8 @@ export interface FacadeOptions { featureLoader?: FeatureLoader; /** Shared services for state sharing across facades (optional) */ sharedServices?: SharedServices; + /** ClaudeUsageService for checking usage limits before picking up features (optional) */ + claudeUsageService?: ClaudeUsageService | null; } /** @@ -110,6 +113,23 @@ export interface OrphanedFeatureInfo { missingBranch: string; } +/** + * Structured error object returned/emitted by facade methods. + * Provides consistent error information for callers and UI consumers. + */ +export interface FacadeError { + /** The facade method where the error originated */ + method: string; + /** Classified error type from the error handler */ + errorType: import('@automaker/types').ErrorType; + /** Human-readable error message */ + message: string; + /** Feature ID if the error is associated with a specific feature */ + featureId?: string; + /** Project path where the error occurred */ + projectPath: string; +} + /** * Interface describing global auto-mode operations (not project-specific). * Used by routes that need global state access. diff --git a/apps/server/src/services/claude-usage-service.ts b/apps/server/src/services/claude-usage-service.ts index aa8afc1c4..6438b5dc4 100644 --- a/apps/server/src/services/claude-usage-service.ts +++ b/apps/server/src/services/claude-usage-service.ts @@ -294,7 +294,16 @@ export class ClaudeUsageService { this.killPtyProcess(ptyProcess); } // Don't fail if we have data - return it instead - if (output.includes('Current session')) { + // Check cleaned output since raw output has ANSI codes between words + // eslint-disable-next-line no-control-regex + const cleanedForCheck = output + .replace(/\x1B\[(\d+)C/g, (_m: string, n: string) => ' '.repeat(parseInt(n, 10))) + .replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, ''); + if ( + cleanedForCheck.includes('Current session') || + cleanedForCheck.includes('% used') || + cleanedForCheck.includes('% left') + ) { resolve(output); } else if (hasSeenTrustPrompt) { // Trust prompt was shown but we couldn't auto-approve it @@ -320,8 +329,13 @@ export class ClaudeUsageService { output += data; // Strip ANSI codes for easier matching + // Convert cursor forward (ESC[nC) to spaces first to preserve word boundaries, + // then strip remaining ANSI sequences. Without this, the Claude CLI TUI output + // like "Current week (all models)" becomes "Currentweek(allmodels)". // eslint-disable-next-line no-control-regex - const cleanOutput = output.replace(/\x1B\[[0-9;]*[A-Za-z]/g, ''); + const cleanOutput = output + .replace(/\x1B\[(\d+)C/g, (_match: string, n: string) => ' '.repeat(parseInt(n, 10))) + .replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, ''); // Check for specific authentication/permission errors // Must be very specific to avoid false positives from garbled terminal encoding @@ -356,7 +370,8 @@ export class ClaudeUsageService { const hasUsageIndicators = cleanOutput.includes('Current session') || (cleanOutput.includes('Usage') && cleanOutput.includes('% left')) || - // Additional patterns for winpty - look for percentage patterns + // Look for percentage patterns - allow optional whitespace between % and left/used + // since cursor movement codes may or may not create spaces after stripping /\d+%\s*(left|used|remaining)/i.test(cleanOutput) || cleanOutput.includes('Resets in') || cleanOutput.includes('Current week'); @@ -382,12 +397,15 @@ export class ClaudeUsageService { // Handle Trust Dialog - multiple variants: // - "Do you want to work in this folder?" // - "Ready to code here?" / "I'll need permission to work with your files" + // - "Quick safety check" / "Yes, I trust this folder" // Since we are running in cwd (project dir), it is safe to approve. if ( !hasApprovedTrust && (cleanOutput.includes('Do you want to work in this folder?') || cleanOutput.includes('Ready to code here') || - cleanOutput.includes('permission to work with your files')) + cleanOutput.includes('permission to work with your files') || + cleanOutput.includes('trust this folder') || + cleanOutput.includes('safety check')) ) { hasApprovedTrust = true; hasSeenTrustPrompt = true; @@ -471,10 +489,17 @@ export class ClaudeUsageService { * Handles CSI, OSC, and other common ANSI sequences */ private stripAnsiCodes(text: string): string { - // First strip ANSI sequences (colors, etc) and handle CR + // First, convert cursor movement sequences to whitespace to preserve word boundaries. + // The Claude CLI TUI uses ESC[nC (cursor forward) instead of actual spaces between words. + // Without this, "Current week (all models)" becomes "Currentweek(allmodels)" after stripping. // eslint-disable-next-line no-control-regex let clean = text - // CSI sequences: ESC [ ... (letter or @) + // Cursor forward (CSI n C): replace with n spaces to preserve word separation + .replace(/\x1B\[(\d+)C/g, (_match, n) => ' '.repeat(parseInt(n, 10))) + // Cursor movement (up/down/back/position): replace with newline or nothing + .replace(/\x1B\[\d*[ABD]/g, '') // cursor up (A), down (B), back (D) + .replace(/\x1B\[\d+;\d+[Hf]/g, '\n') // cursor position (H/f) + // Now strip remaining CSI sequences (colors, modes, etc.) .replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, '') // OSC sequences: ESC ] ... terminated by BEL, ST, or another ESC .replace(/\x1B\][^\x07\x1B]*(?:\x07|\x1B\\)?/g, '') diff --git a/apps/server/src/services/feature-state-manager.ts b/apps/server/src/services/feature-state-manager.ts index cd35859ea..e8afe0b91 100644 --- a/apps/server/src/services/feature-state-manager.ts +++ b/apps/server/src/services/feature-state-manager.ts @@ -107,6 +107,47 @@ export class FeatureStateManager { // Badge will show for 2 minutes after this timestamp if (status === 'waiting_approval') { feature.justFinishedAt = new Date().toISOString(); + + // Finalize task statuses when feature is done: + // - Mark any in_progress tasks as completed (agent finished but didn't explicitly complete them) + // - Do NOT mark pending tasks as completed (they were never started) + // - Clear currentTaskId since no task is actively running + // This prevents cards in "waiting for review" from appearing to still have running tasks + if (feature.planSpec?.tasks) { + let tasksFinalized = 0; + for (const task of feature.planSpec.tasks) { + if (task.status === 'in_progress') { + task.status = 'completed'; + tasksFinalized++; + } + } + if (tasksFinalized > 0) { + logger.info( + `[updateFeatureStatus] Finalized ${tasksFinalized} in_progress tasks for feature ${featureId} moving to waiting_approval` + ); + } + // Update tasksCompleted count to reflect actual completed tasks + feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter( + (t) => t.status === 'completed' + ).length; + feature.planSpec.currentTaskId = undefined; + } + } else if (status === 'verified') { + // Also finalize in_progress tasks when moving directly to verified (skipTests=false) + // Do NOT mark pending tasks as completed - they were never started + if (feature.planSpec?.tasks) { + for (const task of feature.planSpec.tasks) { + if (task.status === 'in_progress') { + task.status = 'completed'; + } + } + feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter( + (t) => t.status === 'completed' + ).length; + feature.planSpec.currentTaskId = undefined; + } + // Clear the timestamp when moving to other statuses + feature.justFinishedAt = undefined; } else { // Clear the timestamp when moving to other statuses feature.justFinishedAt = undefined; diff --git a/apps/server/tests/unit/services/claude-usage-service.test.ts b/apps/server/tests/unit/services/claude-usage-service.test.ts index 7901192c9..bb88381ef 100644 --- a/apps/server/tests/unit/services/claude-usage-service.test.ts +++ b/apps/server/tests/unit/services/claude-usage-service.test.ts @@ -177,6 +177,66 @@ describe('claude-usage-service.ts', () => { // BEL is stripped, newlines and tabs preserved expect(result).toBe('Line 1\nLine 2\tTabbed with bell'); }); + + it('should convert cursor forward (ESC[nC) to spaces', () => { + const service = new ClaudeUsageService(); + // Claude CLI TUI uses ESC[1C instead of space between words + const input = 'Current\x1B[1Csession'; + // @ts-expect-error - accessing private method for testing + const result = service.stripAnsiCodes(input); + + expect(result).toBe('Current session'); + }); + + it('should handle multi-character cursor forward sequences', () => { + const service = new ClaudeUsageService(); + // ESC[3C = move cursor forward 3 positions = 3 spaces + const input = 'Hello\x1B[3Cworld'; + // @ts-expect-error - accessing private method for testing + const result = service.stripAnsiCodes(input); + + expect(result).toBe('Hello world'); + }); + + it('should handle real Claude CLI TUI output with cursor movement codes', () => { + const service = new ClaudeUsageService(); + // Simulates actual Claude CLI /usage output where words are separated by ESC[1C + const input = + 'Current\x1B[1Cweek\x1B[1C(all\x1B[1Cmodels)\n' + + '\x1B[32m█████████████████████████▌\x1B[0m\x1B[1C51%\x1B[1Cused\n' + + 'Resets\x1B[1CFeb\x1B[1C19\x1B[1Cat\x1B[1C3pm\x1B[1C(America/Los_Angeles)'; + // @ts-expect-error - accessing private method for testing + const result = service.stripAnsiCodes(input); + + expect(result).toContain('Current week (all models)'); + expect(result).toContain('51% used'); + expect(result).toContain('Resets Feb 19 at 3pm (America/Los_Angeles)'); + }); + + it('should parse usage output with cursor movement codes between words', () => { + const service = new ClaudeUsageService(); + // Simulates the full /usage TUI output with ESC[1C between every word + const output = + 'Current\x1B[1Csession\n' + + '\x1B[32m█████████████▌\x1B[0m\x1B[1C27%\x1B[1Cused\n' + + 'Resets\x1B[1C9pm\x1B[1C(America/Los_Angeles)\n' + + '\n' + + 'Current\x1B[1Cweek\x1B[1C(all\x1B[1Cmodels)\n' + + '\x1B[32m█████████████████████████▌\x1B[0m\x1B[1C51%\x1B[1Cused\n' + + 'Resets\x1B[1CFeb\x1B[1C19\x1B[1Cat\x1B[1C3pm\x1B[1C(America/Los_Angeles)\n' + + '\n' + + 'Current\x1B[1Cweek\x1B[1C(Sonnet\x1B[1Conly)\n' + + '\x1B[32m██▌\x1B[0m\x1B[1C5%\x1B[1Cused\n' + + 'Resets\x1B[1CFeb\x1B[1C19\x1B[1Cat\x1B[1C11pm\x1B[1C(America/Los_Angeles)'; + // @ts-expect-error - accessing private method for testing + const result = service.parseUsageOutput(output); + + expect(result.sessionPercentage).toBe(27); + expect(result.weeklyPercentage).toBe(51); + expect(result.sonnetWeeklyPercentage).toBe(5); + expect(result.weeklyResetText).toContain('Resets Feb 19 at 3pm'); + expect(result.weeklyResetText).not.toContain('America/Los_Angeles'); + }); }); describe('parseResetTime', () => { diff --git a/apps/server/tests/unit/services/feature-state-manager.test.ts b/apps/server/tests/unit/services/feature-state-manager.test.ts index d53c40b93..bff51d788 100644 --- a/apps/server/tests/unit/services/feature-state-manager.test.ts +++ b/apps/server/tests/unit/services/feature-state-manager.test.ts @@ -151,6 +151,96 @@ describe('FeatureStateManager', () => { expect(savedFeature.justFinishedAt).toBeUndefined(); }); + it('should finalize in_progress and pending tasks when moving to waiting_approval', async () => { + const featureWithTasks: Feature = { + ...mockFeature, + status: 'in_progress', + planSpec: { + status: 'approved', + version: 1, + reviewedByUser: true, + currentTaskId: 'task-2', + tasksCompleted: 1, + tasks: [ + { id: 'task-1', title: 'Task 1', status: 'completed', description: 'First task' }, + { id: 'task-2', title: 'Task 2', status: 'in_progress', description: 'Second task' }, + { id: 'task-3', title: 'Task 3', status: 'pending', description: 'Third task' }, + ], + }, + }; + + (readJsonWithRecovery as Mock).mockResolvedValue({ + data: featureWithTasks, + recovered: false, + source: 'main', + }); + + await manager.updateFeatureStatus('/project', 'feature-123', 'waiting_approval'); + + const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; + // All tasks should be completed + expect(savedFeature.planSpec?.tasks?.[0].status).toBe('completed'); + expect(savedFeature.planSpec?.tasks?.[1].status).toBe('completed'); + expect(savedFeature.planSpec?.tasks?.[2].status).toBe('completed'); + // currentTaskId should be cleared + expect(savedFeature.planSpec?.currentTaskId).toBeUndefined(); + // tasksCompleted should equal total tasks + expect(savedFeature.planSpec?.tasksCompleted).toBe(3); + }); + + it('should finalize tasks when moving to verified status', async () => { + const featureWithTasks: Feature = { + ...mockFeature, + status: 'in_progress', + planSpec: { + status: 'approved', + version: 1, + reviewedByUser: true, + currentTaskId: 'task-2', + tasksCompleted: 1, + tasks: [ + { id: 'task-1', title: 'Task 1', status: 'completed', description: 'First task' }, + { id: 'task-2', title: 'Task 2', status: 'in_progress', description: 'Second task' }, + { id: 'task-3', title: 'Task 3', status: 'pending', description: 'Third task' }, + ], + }, + }; + + (readJsonWithRecovery as Mock).mockResolvedValue({ + data: featureWithTasks, + recovered: false, + source: 'main', + }); + + await manager.updateFeatureStatus('/project', 'feature-123', 'verified'); + + const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; + // All tasks should be completed + expect(savedFeature.planSpec?.tasks?.[0].status).toBe('completed'); + expect(savedFeature.planSpec?.tasks?.[1].status).toBe('completed'); + expect(savedFeature.planSpec?.tasks?.[2].status).toBe('completed'); + // currentTaskId should be cleared + expect(savedFeature.planSpec?.currentTaskId).toBeUndefined(); + // tasksCompleted should equal total tasks + expect(savedFeature.planSpec?.tasksCompleted).toBe(3); + // justFinishedAt should be cleared for verified + expect(savedFeature.justFinishedAt).toBeUndefined(); + }); + + it('should handle waiting_approval without planSpec tasks gracefully', async () => { + (readJsonWithRecovery as Mock).mockResolvedValue({ + data: { ...mockFeature }, + recovered: false, + source: 'main', + }); + + await manager.updateFeatureStatus('/project', 'feature-123', 'waiting_approval'); + + const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; + expect(savedFeature.status).toBe('waiting_approval'); + expect(savedFeature.justFinishedAt).toBeDefined(); + }); + it('should create notification for waiting_approval status', async () => { const mockNotificationService = { createNotification: vi.fn() }; (getNotificationService as Mock).mockReturnValue(mockNotificationService); diff --git a/apps/ui/src/components/ui/task-progress-panel.tsx b/apps/ui/src/components/ui/task-progress-panel.tsx index f72d6174c..6ed402957 100644 --- a/apps/ui/src/components/ui/task-progress-panel.tsx +++ b/apps/ui/src/components/ui/task-progress-panel.tsx @@ -59,24 +59,19 @@ export function TaskProgressPanel({ const planSpec = feature.planSpec; const planTasks = planSpec.tasks; // Already guarded by the if condition above const currentId = planSpec.currentTaskId; - const completedCount = planSpec.tasksCompleted || 0; - // Convert planSpec tasks to TaskInfo with proper status + // Convert planSpec tasks to TaskInfo using their persisted status // planTasks is guaranteed to be defined due to the if condition check - const initialTasks: TaskInfo[] = (planTasks as ParsedTask[]).map( - (t: ParsedTask, index: number) => ({ - id: t.id, - description: t.description, - filePath: t.filePath, - phase: t.phase, - status: - index < completedCount - ? ('completed' as const) - : t.id === currentId - ? ('in_progress' as const) - : ('pending' as const), - }) - ); + const initialTasks: TaskInfo[] = (planTasks as ParsedTask[]).map((t: ParsedTask) => ({ + id: t.id, + description: t.description, + filePath: t.filePath, + phase: t.phase, + status: + t.id === currentId + ? ('in_progress' as const) + : (t.status as TaskInfo['status']) || ('pending' as const), + })); setTasks(initialTasks); setCurrentTaskId(currentId || null); @@ -113,16 +108,12 @@ export function TaskProgressPanel({ const existingIndex = prev.findIndex((t) => t.id === taskEvent.taskId); if (existingIndex !== -1) { - // Update status to in_progress and mark previous as completed - return prev.map((t, idx) => { + // Update only the started task to in_progress + // Do NOT assume previous tasks are completed - rely on actual task_complete events + return prev.map((t) => { if (t.id === taskEvent.taskId) { return { ...t, status: 'in_progress' as const }; } - // If we are moving to a task that is further down the list, assume previous ones are completed - // This is a heuristic, but usually correct for sequential execution - if (idx < existingIndex && t.status !== 'completed') { - return { ...t, status: 'completed' as const }; - } return t; }); } @@ -151,6 +142,24 @@ export function TaskProgressPanel({ setCurrentTaskId(null); } break; + + case 'auto_mode_task_status': + if ('taskId' in event && 'status' in event) { + const taskEvent = event as Extract; + setTasks((prev) => + prev.map((t) => + t.id === taskEvent.taskId + ? { ...t, status: taskEvent.status as TaskInfo['status'] } + : t + ) + ); + if (taskEvent.status === 'in_progress') { + setCurrentTaskId(taskEvent.taskId); + } else if (taskEvent.status === 'completed') { + setCurrentTaskId((current) => (current === taskEvent.taskId ? null : current)); + } + } + break; } }); diff --git a/apps/ui/src/components/usage-popover.tsx b/apps/ui/src/components/usage-popover.tsx index 5d8acb0ba..b3f4347f9 100644 --- a/apps/ui/src/components/usage-popover.tsx +++ b/apps/ui/src/components/usage-popover.tsx @@ -8,6 +8,7 @@ import { cn } from '@/lib/utils'; import { useSetupStore } from '@/store/setup-store'; import { AnthropicIcon, OpenAIIcon } from '@/components/ui/provider-icon'; import { useClaudeUsage, useCodexUsage } from '@/hooks/queries'; +import { getExpectedWeeklyPacePercentage, getPaceStatusLabel } from '@/store/utils/usage-utils'; // Error codes for distinguishing failure modes const ERROR_CODES = { @@ -146,13 +147,28 @@ export function UsagePopover() { return { color: 'text-green-500', icon: CheckCircle, bg: 'bg-green-500' }; }; - // Helper component for the progress bar - const ProgressBar = ({ percentage, colorClass }: { percentage: number; colorClass: string }) => ( -
+ // Helper component for the progress bar with optional pace indicator + const ProgressBar = ({ + percentage, + colorClass, + pacePercentage, + }: { + percentage: number; + colorClass: string; + pacePercentage?: number | null; + }) => ( +
+ {pacePercentage != null && pacePercentage > 0 && pacePercentage < 100 && ( +
+ )}
); @@ -163,6 +179,7 @@ export function UsagePopover() { resetText, isPrimary = false, stale = false, + pacePercentage, }: { title: string; subtitle: string; @@ -170,6 +187,7 @@ export function UsagePopover() { resetText?: string; isPrimary?: boolean; stale?: boolean; + pacePercentage?: number | null; }) => { const isValidPercentage = typeof percentage === 'number' && !isNaN(percentage) && isFinite(percentage); @@ -177,6 +195,10 @@ export function UsagePopover() { const status = getStatusInfo(safePercentage); const StatusIcon = status.icon; + const paceLabel = + isValidPercentage && pacePercentage != null + ? getPaceStatusLabel(safePercentage, pacePercentage) + : null; return (
- {resetText && ( -
+
+ {paceLabel ? ( +

(pacePercentage ?? 0) ? 'text-orange-500' : 'text-green-500' + )} + > + {paceLabel} +

+ ) : ( +
+ )} + {resetText && (

{resetText}

-
- )} + )} +
); }; @@ -384,6 +419,7 @@ export function UsagePopover() { percentage={claudeUsage.sonnetWeeklyPercentage} resetText={claudeUsage.sonnetResetText} stale={isClaudeStale} + pacePercentage={getExpectedWeeklyPacePercentage(claudeUsage.weeklyResetTime)} />
diff --git a/apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx b/apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx index a3540cd75..80e423710 100644 --- a/apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx +++ b/apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx @@ -153,6 +153,7 @@ export const AgentInfoPanel = memo(function AgentInfoPanel({ // Derive effective todos from planSpec.tasks when available, fallback to agentInfo.todos // Uses freshPlanSpec (from API) for accurate progress, with taskStatusMap for real-time updates + const isFeatureFinished = feature.status === 'waiting_approval' || feature.status === 'verified'; const effectiveTodos = useMemo(() => { // Use freshPlanSpec if available (fetched from API), fallback to store's feature.planSpec const planSpec = freshPlanSpec?.tasks?.length ? freshPlanSpec : feature.planSpec; @@ -163,6 +164,16 @@ export const AgentInfoPanel = memo(function AgentInfoPanel({ const currentTaskId = planSpec.currentTaskId; return planSpec.tasks.map((task: ParsedTask, index: number) => { + // If the feature is done (waiting_approval/verified), all tasks are completed + // This is a defensive UI-side check: the server should have already finalized + // task statuses, but stale data from before the fix could still show spinners + if (isFeatureFinished) { + return { + content: task.description, + status: 'completed' as const, + }; + } + // Use real-time status from WebSocket events if available const realtimeStatus = taskStatusMap.get(task.id); @@ -199,6 +210,7 @@ export const AgentInfoPanel = memo(function AgentInfoPanel({ feature.planSpec?.currentTaskId, agentInfo?.todos, taskStatusMap, + isFeatureFinished, ]); // Listen to WebSocket events for real-time task status updates diff --git a/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx b/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx index bdf028b9e..cc97b202c 100644 --- a/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx +++ b/apps/ui/src/components/views/board-view/components/kanban-card/card-header.tsx @@ -1,5 +1,6 @@ // @ts-nocheck - header component props with optional handlers and status variants import { memo, useState } from 'react'; +import type { DraggableAttributes, DraggableSyntheticListeners } from '@dnd-kit/core'; import { Feature } from '@/store/app-store'; import { cn } from '@/lib/utils'; import { CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; @@ -35,6 +36,8 @@ interface CardHeaderProps { onDelete: () => void; onViewOutput?: () => void; onSpawnTask?: () => void; + dragHandleListeners?: DraggableSyntheticListeners; + dragHandleAttributes?: DraggableAttributes; } export const CardHeaderSection = memo(function CardHeaderSection({ @@ -46,6 +49,8 @@ export const CardHeaderSection = memo(function CardHeaderSection({ onDelete, onViewOutput, onSpawnTask, + dragHandleListeners, + dragHandleAttributes, }: CardHeaderProps) { const [isDescriptionExpanded, setIsDescriptionExpanded] = useState(false); const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false); @@ -319,8 +324,10 @@ export const CardHeaderSection = memo(function CardHeaderSection({
{isDraggable && (
diff --git a/apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx b/apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx index 59bf8d8e9..eb44c49b5 100644 --- a/apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx +++ b/apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx @@ -32,7 +32,7 @@ function getCursorClass( ): string { if (isSelectionMode) return 'cursor-pointer'; if (isOverlay) return 'cursor-grabbing'; - if (isDraggable) return 'cursor-grab active:cursor-grabbing'; + // Drag cursor is now only on the drag handle, not the full card return 'cursor-default'; } @@ -172,7 +172,7 @@ export const KanbanCard = memo(function KanbanCard({ const isSelectable = isSelectionMode && feature.status === selectionTarget; const wrapperClasses = cn( - 'relative select-none outline-none touch-none transition-transform duration-200 ease-out', + 'relative select-none outline-none transition-transform duration-200 ease-out', getCursorClass(isOverlay, isDraggable, isSelectable), isOverlay && isLifted && 'scale-105 rotate-1 z-50', // Visual feedback when another card is being dragged over this one @@ -254,6 +254,8 @@ export const KanbanCard = memo(function KanbanCard({ onDelete={onDelete} onViewOutput={onViewOutput} onSpawnTask={onSpawnTask} + dragHandleListeners={isDraggable ? listeners : undefined} + dragHandleAttributes={isDraggable ? attributes : undefined} /> @@ -296,8 +298,6 @@ export const KanbanCard = memo(function KanbanCard({
diff --git a/apps/ui/src/components/views/board-view/mobile-usage-bar.tsx b/apps/ui/src/components/views/board-view/mobile-usage-bar.tsx index 918988e91..687de7859 100644 --- a/apps/ui/src/components/views/board-view/mobile-usage-bar.tsx +++ b/apps/ui/src/components/views/board-view/mobile-usage-bar.tsx @@ -5,6 +5,7 @@ import { Spinner } from '@/components/ui/spinner'; import { getElectronAPI } from '@/lib/electron'; import { useAppStore } from '@/store/app-store'; import { AnthropicIcon, OpenAIIcon } from '@/components/ui/provider-icon'; +import { getExpectedWeeklyPacePercentage, getPaceStatusLabel } from '@/store/utils/usage-utils'; interface MobileUsageBarProps { showClaudeUsage: boolean; @@ -23,11 +24,15 @@ function UsageBar({ label, percentage, isStale, + pacePercentage, }: { label: string; percentage: number; isStale: boolean; + pacePercentage?: number | null; }) { + const paceLabel = pacePercentage != null ? getPaceStatusLabel(percentage, pacePercentage) : null; + return (
@@ -49,7 +54,7 @@ function UsageBar({
@@ -57,7 +62,24 @@ function UsageBar({ className={cn('h-full transition-all duration-500', getProgressBarColor(percentage))} style={{ width: `${Math.min(percentage, 100)}%` }} /> + {pacePercentage != null && pacePercentage > 0 && pacePercentage < 100 && ( +
+ )}
+ {paceLabel && ( +

(pacePercentage ?? 0) ? 'text-orange-500' : 'text-green-500' + )} + > + {paceLabel} +

+ )}
); } @@ -190,6 +212,7 @@ export function MobileUsageBar({ showClaudeUsage, showCodexUsage }: MobileUsageB label="Weekly" percentage={claudeUsage.weeklyPercentage} isStale={isClaudeStale} + pacePercentage={getExpectedWeeklyPacePercentage(claudeUsage.weeklyResetTime)} /> ) : ( diff --git a/apps/ui/src/store/utils/usage-utils.ts b/apps/ui/src/store/utils/usage-utils.ts index 7b82fb128..1c363f217 100644 --- a/apps/ui/src/store/utils/usage-utils.ts +++ b/apps/ui/src/store/utils/usage-utils.ts @@ -1,5 +1,64 @@ import type { ClaudeUsage } from '../types/usage-types'; +/** + * Calculate the expected weekly usage percentage based on how far through the week we are. + * Claude's weekly usage resets every Thursday. Given the reset time (when the NEXT reset occurs), + * we can determine how much of the week has elapsed and therefore what percentage of the budget + * should have been used if usage were evenly distributed. + * + * @param weeklyResetTime - ISO date string for when the weekly usage next resets + * @returns The expected usage percentage (0-100), or null if the reset time is invalid + */ +export function getExpectedWeeklyPacePercentage( + weeklyResetTime: string | undefined +): number | null { + if (!weeklyResetTime) return null; + + try { + const resetDate = new Date(weeklyResetTime); + if (isNaN(resetDate.getTime())) return null; + + const now = new Date(); + const WEEK_MS = 7 * 24 * 60 * 60 * 1000; + + // The week started 7 days before the reset + const weekStartDate = new Date(resetDate.getTime() - WEEK_MS); + + // How far through the week are we? + const elapsed = now.getTime() - weekStartDate.getTime(); + const fractionElapsed = elapsed / WEEK_MS; + + // Clamp to 0-1 range + const clamped = Math.max(0, Math.min(1, fractionElapsed)); + + return clamped * 100; + } catch { + return null; + } +} + +/** + * Get a human-readable label for the pace status (ahead or behind expected usage). + * + * @param actualPercentage - The actual usage percentage (0-100) + * @param expectedPercentage - The expected usage percentage (0-100) + * @returns A string like "5% ahead of pace" or "10% behind pace", or null + */ +export function getPaceStatusLabel( + actualPercentage: number, + expectedPercentage: number | null +): string | null { + if (expectedPercentage === null) return null; + + const diff = Math.round(actualPercentage - expectedPercentage); + + if (diff === 0) return 'On pace'; + // Using more than expected = behind pace (bad) + if (diff > 0) return `${Math.abs(diff)}% behind pace`; + // Using less than expected = ahead of pace (good) + return `${Math.abs(diff)}% ahead of pace`; +} + /** * Check if Claude usage is at its limit (any of: session >= 100%, weekly >= 100%, OR cost >= limit) * Returns true if any limit is reached, meaning auto mode should pause feature pickup. From 4a8c6b0eba0a3c32961f8a9edf2555842b6b1517 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 16 Feb 2026 20:47:38 -0800 Subject: [PATCH 2/3] Update feature-state-manager.test.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../tests/unit/services/feature-state-manager.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/server/tests/unit/services/feature-state-manager.test.ts b/apps/server/tests/unit/services/feature-state-manager.test.ts index bff51d788..65998ce1b 100644 --- a/apps/server/tests/unit/services/feature-state-manager.test.ts +++ b/apps/server/tests/unit/services/feature-state-manager.test.ts @@ -178,14 +178,14 @@ describe('FeatureStateManager', () => { await manager.updateFeatureStatus('/project', 'feature-123', 'waiting_approval'); const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; - // All tasks should be completed + // Only in_progress tasks should be completed expect(savedFeature.planSpec?.tasks?.[0].status).toBe('completed'); expect(savedFeature.planSpec?.tasks?.[1].status).toBe('completed'); - expect(savedFeature.planSpec?.tasks?.[2].status).toBe('completed'); + expect(savedFeature.planSpec?.tasks?.[2].status).toBe('pending'); // currentTaskId should be cleared expect(savedFeature.planSpec?.currentTaskId).toBeUndefined(); - // tasksCompleted should equal total tasks - expect(savedFeature.planSpec?.tasksCompleted).toBe(3); + // tasksCompleted should be 2, not 3 + expect(savedFeature.planSpec?.tasksCompleted).toBe(2); }); it('should finalize tasks when moving to verified status', async () => { From 30fce3f7469bbd9ab04d2ae85be2dabb769df23e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 16 Feb 2026 21:24:24 -0800 Subject: [PATCH 3/3] test: Update task finalization behavior to keep pending tasks in review states --- .../services/feature-state-manager.test.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/server/tests/unit/services/feature-state-manager.test.ts b/apps/server/tests/unit/services/feature-state-manager.test.ts index 65998ce1b..6abd4764d 100644 --- a/apps/server/tests/unit/services/feature-state-manager.test.ts +++ b/apps/server/tests/unit/services/feature-state-manager.test.ts @@ -151,7 +151,7 @@ describe('FeatureStateManager', () => { expect(savedFeature.justFinishedAt).toBeUndefined(); }); - it('should finalize in_progress and pending tasks when moving to waiting_approval', async () => { + it('should finalize in_progress tasks but keep pending tasks when moving to waiting_approval', async () => { const featureWithTasks: Feature = { ...mockFeature, status: 'in_progress', @@ -178,13 +178,15 @@ describe('FeatureStateManager', () => { await manager.updateFeatureStatus('/project', 'feature-123', 'waiting_approval'); const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; - // Only in_progress tasks should be completed + // Already completed tasks stay completed expect(savedFeature.planSpec?.tasks?.[0].status).toBe('completed'); + // in_progress tasks should be finalized to completed expect(savedFeature.planSpec?.tasks?.[1].status).toBe('completed'); + // pending tasks should remain pending (never started) expect(savedFeature.planSpec?.tasks?.[2].status).toBe('pending'); // currentTaskId should be cleared expect(savedFeature.planSpec?.currentTaskId).toBeUndefined(); - // tasksCompleted should be 2, not 3 + // tasksCompleted should equal actual completed tasks count expect(savedFeature.planSpec?.tasksCompleted).toBe(2); }); @@ -215,14 +217,16 @@ describe('FeatureStateManager', () => { await manager.updateFeatureStatus('/project', 'feature-123', 'verified'); const savedFeature = (atomicWriteJson as Mock).mock.calls[0][1] as Feature; - // All tasks should be completed + // Already completed tasks stay completed expect(savedFeature.planSpec?.tasks?.[0].status).toBe('completed'); + // in_progress tasks should be finalized to completed expect(savedFeature.planSpec?.tasks?.[1].status).toBe('completed'); - expect(savedFeature.planSpec?.tasks?.[2].status).toBe('completed'); + // pending tasks should remain pending (never started) + expect(savedFeature.planSpec?.tasks?.[2].status).toBe('pending'); // currentTaskId should be cleared expect(savedFeature.planSpec?.currentTaskId).toBeUndefined(); - // tasksCompleted should equal total tasks - expect(savedFeature.planSpec?.tasksCompleted).toBe(3); + // tasksCompleted should equal actual completed tasks count + expect(savedFeature.planSpec?.tasksCompleted).toBe(2); // justFinishedAt should be cleared for verified expect(savedFeature.justFinishedAt).toBeUndefined(); });