fix: address PR review issues for auto-mode refactor

- agent-executor: move executeQuery into try block for proper heartbeat cleanup,
  re-parse tasks when edited plan is approved
- auto-loop-coordinator: handle feature execution failures with proper logging
  and failure tracking, support backward-compatible method signatures
- facade: delegate getActiveAutoLoopProjects/Worktrees to coordinator,
  always create own AutoLoopCoordinator (not shared), pass projectPath
  to approval methods and branchName to failure tracking
- global-service: document shared autoLoopCoordinator is for monitoring only
- execution-types: fix ExecuteFeatureFn type to match implementation
- feature-state-manager: use readJsonWithRecovery for loadFeature
- pipeline-orchestrator: add defensive null check and try/catch for
  merge response parsing
- plan-approval-service: use project-scoped keys to prevent cross-project
  collisions, maintain backward compatibility for featureId-only lookups

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

apps/server/src/services/agent-executor.ts

@@ -180,9 +180,9 @@ export class AgentExecutor {
     }
 
     logger.info(`Starting stream for feature ${featureId}...`);
-    const stream = provider.executeQuery(executeOptions);
 
     try {
+      const stream = provider.executeQuery(executeOptions);
       streamLoop: for await (const msg of stream) {
         receivedAnyStreamMessage = true;
         appendRawEvent(msg);
@@ -255,7 +255,7 @@ export class AgentExecutor {
                 input: block.input,
               });
               if (responseText.length > 0 && !responseText.endsWith('\n')) responseText += '\n';
-              responseText += `\n Tool: ${block.name}\n`;
+              responseText += `\n🔧 Tool: ${block.name}\n`;
               if (block.input) responseText += `Input: ${JSON.stringify(block.input, null, 2)}\n`;
               scheduleWrite();
             }
@@ -502,10 +502,17 @@ export class AgentExecutor {
           planApproved = true;
           userFeedback = approvalResult.feedback;
           approvedPlanContent = approvalResult.editedPlan || currentPlanContent;
-          if (approvalResult.editedPlan)
+          if (approvalResult.editedPlan) {
+            // Re-parse tasks from edited plan to ensure we execute the updated tasks
+            const editedTasks = parseTasksFromSpec(approvalResult.editedPlan);
+            parsedTasks = editedTasks;
             await this.featureStateManager.updateFeaturePlanSpec(projectPath, featureId, {
               content: approvalResult.editedPlan,
+              tasks: editedTasks,
+              tasksTotal: editedTasks.length,
+              tasksCompleted: 0,
             });
+          }
           this.eventBus.emitAutoModeEvent('plan_approved', {
             featureId,
             projectPath,

apps/server/src/services/auto-loop-coordinator.ts

@@ -183,7 +183,13 @@ export class AutoLoopCoordinator {
             nextFeature.id,
             projectState.config.useWorktrees,
             true
-          ).catch(() => {});
+          ).catch((error) => {
+            const errorInfo = classifyError(error);
+            logger.error(`Auto-loop feature ${nextFeature.id} failed:`, errorInfo.message);
+            if (this.trackFailureAndCheckPauseForProject(projectPath, branchName, errorInfo)) {
+              this.signalShouldPauseForProject(projectPath, branchName, errorInfo);
+            }
+          });
         }
         await this.sleep(2000, projectState.abortController.signal);
       } catch {
@@ -268,27 +274,64 @@ export class AutoLoopCoordinator {
 
   trackFailureAndCheckPauseForProject(
     projectPath: string,
-    errorInfo: { type: string; message: string }
+    branchNameOrError: string | null | { type: string; message: string },
+    errorInfo?: { type: string; message: string }
   ): boolean {
-    const projectState = this.autoLoopsByProject.get(getWorktreeAutoLoopKey(projectPath, null));
+    // Support both old (projectPath, errorInfo) and new (projectPath, branchName, errorInfo) signatures
+    let branchName: string | null;
+    let actualErrorInfo: { type: string; message: string };
+    if (
+      typeof branchNameOrError === 'object' &&
+      branchNameOrError !== null &&
+      'type' in branchNameOrError
+    ) {
+      // Old signature: (projectPath, errorInfo)
+      branchName = null;
+      actualErrorInfo = branchNameOrError;
+    } else {
+      // New signature: (projectPath, branchName, errorInfo)
+      branchName = branchNameOrError;
+      actualErrorInfo = errorInfo!;
+    }
+    const projectState = this.autoLoopsByProject.get(
+      getWorktreeAutoLoopKey(projectPath, branchName)
+    );
     if (!projectState) return false;
     const now = Date.now();
-    projectState.consecutiveFailures.push({ timestamp: now, error: errorInfo.message });
+    projectState.consecutiveFailures.push({ timestamp: now, error: actualErrorInfo.message });
     projectState.consecutiveFailures = projectState.consecutiveFailures.filter(
       (f) => now - f.timestamp < FAILURE_WINDOW_MS
     );
     return (
       projectState.consecutiveFailures.length >= CONSECUTIVE_FAILURE_THRESHOLD ||
-      errorInfo.type === 'quota_exhausted' ||
-      errorInfo.type === 'rate_limit'
+      actualErrorInfo.type === 'quota_exhausted' ||
+      actualErrorInfo.type === 'rate_limit'
     );
   }
 
   signalShouldPauseForProject(
     projectPath: string,
-    errorInfo: { type: string; message: string }
+    branchNameOrError: string | null | { type: string; message: string },
+    errorInfo?: { type: string; message: string }
   ): void {
-    const projectState = this.autoLoopsByProject.get(getWorktreeAutoLoopKey(projectPath, null));
+    // Support both old (projectPath, errorInfo) and new (projectPath, branchName, errorInfo) signatures
+    let branchName: string | null;
+    let actualErrorInfo: { type: string; message: string };
+    if (
+      typeof branchNameOrError === 'object' &&
+      branchNameOrError !== null &&
+      'type' in branchNameOrError
+    ) {
+      branchName = null;
+      actualErrorInfo = branchNameOrError;
+    } else {
+      branchName = branchNameOrError;
+      actualErrorInfo = errorInfo!;
+    }
+
+    const projectState = this.autoLoopsByProject.get(
+      getWorktreeAutoLoopKey(projectPath, branchName)
+    );
     if (!projectState || projectState.pausedDueToFailures) return;
     projectState.pausedDueToFailures = true;
     const failureCount = projectState.consecutiveFailures.length;
@@ -297,24 +340,29 @@ export class AutoLoopCoordinator {
         failureCount >= CONSECUTIVE_FAILURE_THRESHOLD
           ? `Auto Mode paused: ${failureCount} consecutive failures detected.`
           : 'Auto Mode paused: Usage limit or API error detected.',
-      errorType: errorInfo.type,
-      originalError: errorInfo.message,
+      errorType: actualErrorInfo.type,
+      originalError: actualErrorInfo.message,
       failureCount,
       projectPath,
+      branchName,
     });
-    this.stopAutoLoopForProject(projectPath);
+    this.stopAutoLoopForProject(projectPath, branchName);
   }
 
-  resetFailureTrackingForProject(projectPath: string): void {
-    const projectState = this.autoLoopsByProject.get(getWorktreeAutoLoopKey(projectPath, null));
+  resetFailureTrackingForProject(projectPath: string, branchName: string | null = null): void {
+    const projectState = this.autoLoopsByProject.get(
+      getWorktreeAutoLoopKey(projectPath, branchName)
+    );
     if (projectState) {
       projectState.consecutiveFailures = [];
       projectState.pausedDueToFailures = false;
     }
   }
 
-  recordSuccessForProject(projectPath: string): void {
-    const projectState = this.autoLoopsByProject.get(getWorktreeAutoLoopKey(projectPath, null));
+  recordSuccessForProject(projectPath: string, branchName: string | null = null): void {
+    const projectState = this.autoLoopsByProject.get(
+      getWorktreeAutoLoopKey(projectPath, branchName)
+    );
     if (projectState) projectState.consecutiveFailures = [];
   }
 

apps/server/src/services/auto-mode/facade.ts

@@ -14,8 +14,8 @@
 import path from 'path';
 import { exec } from 'child_process';
 import { promisify } from 'util';
-import type { Feature } from '@automaker/types';
-import { DEFAULT_MAX_CONCURRENCY } from '@automaker/types';
+import type { Feature, PlanningMode, ThinkingLevel } from '@automaker/types';
+import { DEFAULT_MAX_CONCURRENCY, stripProviderPrefix } from '@automaker/types';
 import { createLogger, loadContextFiles, classifyError } from '@automaker/utils';
 import { getFeatureDir } from '@automaker/platform';
 import * as secureFs from '../../lib/secure-fs.js';
@@ -32,6 +32,7 @@ import { RecoveryService } from '../recovery-service.js';
 import { PipelineOrchestrator } from '../pipeline-orchestrator.js';
 import { AgentExecutor } from '../agent-executor.js';
 import { TestRunnerService } from '../test-runner-service.js';
+import { ProviderFactory } from '../../providers/provider-factory.js';
 import { FeatureLoader } from '../feature-loader.js';
 import type { SettingsService } from '../settings-service.js';
 import type { EventEmitter } from '../../lib/events.js';
@@ -153,55 +154,162 @@ export class AutoModeServiceFacade {
       buildFeaturePrompt,
       (pPath, featureId, useWorktrees, _isAutoMode, _model, opts) =>
         facadeInstance!.executeFeature(featureId, useWorktrees, false, undefined, opts),
-      // runAgentFn stub - facade does not implement runAgent directly
-      async () => {
-        throw new Error('runAgentFn not implemented in facade');
+      // runAgentFn - delegates to AgentExecutor
+      async (
+        workDir: string,
+        featureId: string,
+        prompt: string,
+        abortController: AbortController,
+        pPath: string,
+        imagePaths?: string[],
+        model?: string,
+        opts?: Record<string, unknown>
+      ) => {
+        const resolvedModel = model || 'claude-sonnet-4-20250514';
+        const provider = ProviderFactory.getProviderForModel(resolvedModel);
+        const effectiveBareModel = stripProviderPrefix(resolvedModel);
+
+        await agentExecutor.execute(
+          {
+            workDir,
+            featureId,
+            prompt,
+            projectPath: pPath,
+            abortController,
+            imagePaths,
+            model: resolvedModel,
+            planningMode: opts?.planningMode as PlanningMode | undefined,
+            requirePlanApproval: opts?.requirePlanApproval as boolean | undefined,
+            previousContent: opts?.previousContent as string | undefined,
+            systemPrompt: opts?.systemPrompt as string | undefined,
+            autoLoadClaudeMd: opts?.autoLoadClaudeMd as boolean | undefined,
+            thinkingLevel: opts?.thinkingLevel as ThinkingLevel | undefined,
+            branchName: opts?.branchName as string | null | undefined,
+            provider,
+            effectiveBareModel,
+          },
+          {
+            waitForApproval: (fId, projPath) => planApprovalService.waitForApproval(fId, projPath),
+            saveFeatureSummary: (projPath, fId, summary) =>
+              featureStateManager.saveFeatureSummary(projPath, fId, summary),
+            updateFeatureSummary: (projPath, fId, summary) =>
+              featureStateManager.saveFeatureSummary(projPath, fId, summary),
+            buildTaskPrompt: (task, allTasks, taskIndex, _planContent, template, feedback) => {
+              let taskPrompt = template
+                .replace(/\{\{taskName\}\}/g, task.description)
+                .replace(/\{\{taskIndex\}\}/g, String(taskIndex + 1))
+                .replace(/\{\{totalTasks\}\}/g, String(allTasks.length))
+                .replace(/\{\{taskDescription\}\}/g, task.description || task.description);
+              if (feedback) {
+                taskPrompt = taskPrompt.replace(/\{\{userFeedback\}\}/g, feedback);
+              }
+              return taskPrompt;
+            },
+          }
+        );
       }
     );
 
-    // AutoLoopCoordinator - use shared if provided, otherwise create new
-    // Note: When using shared autoLoopCoordinator, callbacks are already set up by the global service
-    const autoLoopCoordinator =
-      sharedServices?.autoLoopCoordinator ??
-      new AutoLoopCoordinator(
-        eventBus,
-        concurrencyManager,
-        settingsService,
-        // Callbacks
-        (pPath, featureId, useWorktrees, isAutoMode) =>
-          facadeInstance!.executeFeature(featureId, useWorktrees, isAutoMode),
-        (pPath, branchName) =>
-          featureLoader
-            .getAll(pPath)
-            .then((features) =>
-              features.filter(
-                (f) =>
-                  (f.status === 'backlog' || f.status === 'ready') &&
-                  (branchName === null
-                    ? !f.branchName || f.branchName === 'main'
-                    : f.branchName === branchName)
-              )
-            ),
-        (pPath, branchName, maxConcurrency) =>
-          facadeInstance!.saveExecutionStateForProject(branchName, maxConcurrency),
-        (pPath, branchName) => facadeInstance!.clearExecutionState(branchName),
-        (pPath) => featureStateManager.resetStuckFeatures(pPath),
-        (feature) =>
-          feature.status === 'completed' ||
-          feature.status === 'verified' ||
-          feature.status === 'waiting_approval',
-        (featureId) => concurrencyManager.isRunning(featureId)
-      );
+    // AutoLoopCoordinator - ALWAYS create new with proper execution callbacks
+    // NOTE: We don't use sharedServices.autoLoopCoordinator because it doesn't have
+    // execution callbacks. Each facade needs its own coordinator to execute features.
+    // The shared coordinator in GlobalAutoModeService is for monitoring only.
+    const autoLoopCoordinator = new AutoLoopCoordinator(
+      eventBus,
+      concurrencyManager,
+      settingsService,
+      // Callbacks
+      (pPath, featureId, useWorktrees, isAutoMode) =>
+        facadeInstance!.executeFeature(featureId, useWorktrees, isAutoMode),
+      (pPath, branchName) =>
+        featureLoader
+          .getAll(pPath)
+          .then((features) =>
+            features.filter(
+              (f) =>
+                (f.status === 'backlog' || f.status === 'ready') &&
+                (branchName === null
+                  ? !f.branchName || f.branchName === 'main'
+                  : f.branchName === branchName)
+            )
+          ),
+      (pPath, branchName, maxConcurrency) =>
+        facadeInstance!.saveExecutionStateForProject(branchName, maxConcurrency),
+      (pPath, branchName) => facadeInstance!.clearExecutionState(branchName),
+      (pPath) => featureStateManager.resetStuckFeatures(pPath),
+      (feature) =>
+        feature.status === 'completed' ||
+        feature.status === 'verified' ||
+        feature.status === 'waiting_approval',
+      (featureId) => concurrencyManager.isRunning(featureId)
+    );
 
-    // ExecutionService - runAgentFn is a stub
+    // ExecutionService - runAgentFn calls AgentExecutor.execute
     const executionService = new ExecutionService(
       eventBus,
       concurrencyManager,
       worktreeResolver,
       settingsService,
-      // Callbacks - runAgentFn stub
-      async () => {
-        throw new Error('runAgentFn not implemented in facade');
+      // runAgentFn - delegates to AgentExecutor
+      async (
+        workDir: string,
+        featureId: string,
+        prompt: string,
+        abortController: AbortController,
+        pPath: string,
+        imagePaths?: string[],
+        model?: string,
+        opts?: {
+          projectPath?: string;
+          planningMode?: PlanningMode;
+          requirePlanApproval?: boolean;
+          systemPrompt?: string;
+          autoLoadClaudeMd?: boolean;
+          thinkingLevel?: ThinkingLevel;
+          branchName?: string | null;
+        }
+      ) => {
+        const resolvedModel = model || 'claude-sonnet-4-20250514';
+        const provider = ProviderFactory.getProviderForModel(resolvedModel);
+        const effectiveBareModel = stripProviderPrefix(resolvedModel);
+
+        await agentExecutor.execute(
+          {
+            workDir,
+            featureId,
+            prompt,
+            projectPath: pPath,
+            abortController,
+            imagePaths,
+            model: resolvedModel,
+            planningMode: opts?.planningMode,
+            requirePlanApproval: opts?.requirePlanApproval,
+            systemPrompt: opts?.systemPrompt,
+            autoLoadClaudeMd: opts?.autoLoadClaudeMd,
+            thinkingLevel: opts?.thinkingLevel,
+            branchName: opts?.branchName,
+            provider,
+            effectiveBareModel,
+          },
+          {
+            waitForApproval: (fId, projPath) => planApprovalService.waitForApproval(fId, projPath),
+            saveFeatureSummary: (projPath, fId, summary) =>
+              featureStateManager.saveFeatureSummary(projPath, fId, summary),
+            updateFeatureSummary: (projPath, fId, summary) =>
+              featureStateManager.saveFeatureSummary(projPath, fId, summary),
+            buildTaskPrompt: (task, allTasks, taskIndex, planContent, template, feedback) => {
+              let taskPrompt = template
+                .replace(/\{\{taskName\}\}/g, task.description)
+                .replace(/\{\{taskIndex\}\}/g, String(taskIndex + 1))
+                .replace(/\{\{totalTasks\}\}/g, String(allTasks.length))
+                .replace(/\{\{taskDescription\}\}/g, task.description || task.description);
+              if (feedback) {
+                taskPrompt = taskPrompt.replace(/\{\{userFeedback\}\}/g, feedback);
+              }
+              return taskPrompt;
+            },
+          }
+        );
       },
       (context) => pipelineOrchestrator.executePipeline(context),
       (pPath, featureId, status) =>
@@ -220,8 +328,8 @@ export class AutoModeServiceFacade {
       (pPath, featureId, useWorktrees, _calledInternally) =>
         facadeInstance!.resumeFeature(featureId, useWorktrees, _calledInternally),
       (errorInfo) =>
-        autoLoopCoordinator.trackFailureAndCheckPauseForProject(projectPath, errorInfo),
-      (errorInfo) => autoLoopCoordinator.signalShouldPauseForProject(projectPath, errorInfo),
+        autoLoopCoordinator.trackFailureAndCheckPauseForProject(projectPath, null, errorInfo),
+      (errorInfo) => autoLoopCoordinator.signalShouldPauseForProject(projectPath, null, errorInfo),
       () => {
         /* recordSuccess - no-op */
       },
@@ -638,18 +746,14 @@ Address the follow-up instructions above. Review the previous work and make the
    * Get all active auto loop projects (unique project paths)
    */
   getActiveAutoLoopProjects(): string[] {
-    // This needs access to internal state - for now return empty
-    // Routes should migrate to getActiveAutoLoopWorktrees
-    return [];
+    return this.autoLoopCoordinator.getActiveProjects();
   }
 
   /**
    * Get all active auto loop worktrees
    */
   getActiveAutoLoopWorktrees(): Array<{ projectPath: string; branchName: string | null }> {
-    // This needs access to internal state - for now return empty
-    // Will be properly implemented when routes migrate
-    return [];
+    return this.autoLoopCoordinator.getActiveWorktrees();
   }
 
   /**
@@ -783,7 +887,7 @@ Address the follow-up instructions above. Review the previous work and make the
    * @param featureId - The feature ID
    */
   hasPendingApproval(featureId: string): boolean {
-    return this.planApprovalService.hasPendingApproval(featureId);
+    return this.planApprovalService.hasPendingApproval(featureId, this.projectPath);
   }
 
   /**
@@ -791,7 +895,7 @@ Address the follow-up instructions above. Review the previous work and make the
    * @param featureId - The feature ID
    */
   cancelPlanApproval(featureId: string): void {
-    this.planApprovalService.cancelApproval(featureId);
+    this.planApprovalService.cancelApproval(featureId, this.projectPath);
   }
 
   // ===========================================================================

apps/server/src/services/auto-mode/global-service.ts

@@ -51,15 +51,19 @@ export class GlobalAutoModeService {
     this.featureStateManager = new FeatureStateManager(events, featureLoader);
 
     // Create AutoLoopCoordinator with callbacks
-    // These callbacks use placeholders since GlobalAutoModeService doesn't execute features
-    // Feature execution is done via facades
+    // IMPORTANT: This coordinator is for MONITORING ONLY (getActiveProjects, getActiveWorktrees).
+    // Facades MUST create their own AutoLoopCoordinator for actual execution.
+    // The executeFeatureFn here is a safety guard - it should never be called.
     this.autoLoopCoordinator = new AutoLoopCoordinator(
       this.eventBus,
       this.concurrencyManager,
       settingsService,
-      // executeFeatureFn - not used by global service, routes handle execution
+      // executeFeatureFn - throws because facades must use their own coordinator for execution
       async () => {
-        throw new Error('executeFeatureFn not available in GlobalAutoModeService');
+        throw new Error(
+          'executeFeatureFn not available in GlobalAutoModeService. ' +
+            'Facades must create their own AutoLoopCoordinator for execution.'
+        );
       },
       // getBacklogFeaturesFn
       (pPath, branchName) =>

apps/server/src/services/execution-types.ts

@@ -138,9 +138,9 @@ export type ExecuteFeatureFn = (
   projectPath: string,
   featureId: string,
   useWorktrees: boolean,
-  useScreenshots: boolean,
-  model?: string,
-  options?: { _calledInternally?: boolean }
+  isAutoMode: boolean,
+  providedWorktreePath?: string,
+  options?: { continuationPrompt?: string; _calledInternally?: boolean }
 ) => Promise<void>;
 
 /**

apps/server/src/services/feature-state-manager.ts

@@ -60,8 +60,12 @@ export class FeatureStateManager {
     const featurePath = path.join(featureDir, 'feature.json');
 
     try {
-      const data = (await secureFs.readFile(featurePath, 'utf-8')) as string;
-      return JSON.parse(data);
+      const result = await readJsonWithRecovery<Feature | null>(featurePath, null, {
+        maxBackups: DEFAULT_BACKUP_COUNT,
+        autoRestore: true,
+      });
+      logRecoveryWarning(result, `Feature ${featureId}`, logger);
+      return result.data;
     } catch {
       return null;
     }

apps/server/src/services/pipeline-orchestrator.ts

@@ -495,11 +495,23 @@ export class PipelineOrchestrator {
         }),
       });
 
-      const data = (await response.json()) as {
-        success: boolean;
-        hasConflicts?: boolean;
-        error?: string;
-      };
+      if (!response) {
+        return { success: false, error: 'No response from merge endpoint' };
+      }
+
+      // Defensively parse JSON response
+      let data: { success: boolean; hasConflicts?: boolean; error?: string };
+      try {
+        data = (await response.json()) as {
+          success: boolean;
+          hasConflicts?: boolean;
+          error?: string;
+        };
+      } catch (parseError) {
+        logger.error(`Failed to parse merge response:`, parseError);
+        return { success: false, error: 'Invalid response from merge endpoint' };
+      }
+
       if (!response.ok) {
         if (data.hasConflicts) {
           await this.updateFeatureStatusFn(projectPath, featureId, 'merge_conflict');

apps/server/src/services/plan-approval-service.ts

@@ -66,12 +66,18 @@ export class PlanApprovalService {
     this.settingsService = settingsService;
   }
 
+  /** Generate project-scoped key to prevent collisions across projects */
+  private approvalKey(projectPath: string, featureId: string): string {
+    return `${projectPath}::${featureId}`;
+  }
+
   /** Wait for plan approval with timeout (default 30 min). Rejects on timeout/cancellation. */
   async waitForApproval(featureId: string, projectPath: string): Promise<PlanApprovalResult> {
     const timeoutMs = await this.getTimeoutMs(projectPath);
     const timeoutMinutes = Math.round(timeoutMs / 60000);
+    const key = this.approvalKey(projectPath, featureId);
 
-    logger.info(`Registering pending approval for feature ${featureId}`);
+    logger.info(`Registering pending approval for feature ${featureId} in project ${projectPath}`);
     logger.info(
       `Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`
     );
@@ -80,12 +86,12 @@ export class PlanApprovalService {
       // Set up timeout to prevent indefinite waiting and memory leaks
       // timeoutId stored in closure, NOT in PendingApproval object
       const timeoutId = setTimeout(() => {
-        const pending = this.pendingApprovals.get(featureId);
+        const pending = this.pendingApprovals.get(key);
         if (pending) {
           logger.warn(
             `Plan approval for feature ${featureId} timed out after ${timeoutMinutes} minutes`
           );
-          this.pendingApprovals.delete(featureId);
+          this.pendingApprovals.delete(key);
           reject(
             new Error(
               `Plan approval timed out after ${timeoutMinutes} minutes - feature execution cancelled`
@@ -106,7 +112,7 @@ export class PlanApprovalService {
         reject(error);
       };
 
-      this.pendingApprovals.set(featureId, {
+      this.pendingApprovals.set(key, {
         resolve: wrappedResolve,
         reject: wrappedReject,
         featureId,
@@ -132,7 +138,23 @@ export class PlanApprovalService {
       `Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`
     );
 
-    const pending = this.pendingApprovals.get(featureId);
+    // Try to find pending approval using project-scoped key if projectPath is available
+    let foundKey: string | undefined;
+    let pending: PendingApproval | undefined;
+
+    if (projectPathFromClient) {
+      foundKey = this.approvalKey(projectPathFromClient, featureId);
+      pending = this.pendingApprovals.get(foundKey);
+    } else {
+      // Fallback: search by featureId (backward compatibility)
+      for (const [key, approval] of this.pendingApprovals) {
+        if (approval.featureId === featureId) {
+          foundKey = key;
+          pending = approval;
+          break;
+        }
+      }
+    }
 
     if (!pending) {
       logger.info(`No pending approval in Map for feature ${featureId}`);
@@ -219,32 +241,60 @@ export class PlanApprovalService {
     // Resolve the promise with all data including feedback
     // This triggers the wrapped resolve which clears the timeout
     pending.resolve({ approved, editedPlan, feedback });
-    this.pendingApprovals.delete(featureId);
+    if (foundKey) {
+      this.pendingApprovals.delete(foundKey);
+    }
 
     return { success: true };
   }
 
   /** Cancel approval (e.g., when feature stopped). Timeout cleared via wrapped reject. */
-  cancelApproval(featureId: string): void {
+  cancelApproval(featureId: string, projectPath?: string): void {
     logger.info(`cancelApproval called for feature ${featureId}`);
     logger.info(
       `Current pending approvals: ${Array.from(this.pendingApprovals.keys()).join(', ') || 'none'}`
     );
 
-    const pending = this.pendingApprovals.get(featureId);
-    if (pending) {
+    // If projectPath provided, use project-scoped key; otherwise search by featureId
+    let foundKey: string | undefined;
+    let pending: PendingApproval | undefined;
+
+    if (projectPath) {
+      foundKey = this.approvalKey(projectPath, featureId);
+      pending = this.pendingApprovals.get(foundKey);
+    } else {
+      // Fallback: search for any approval with this featureId (backward compatibility)
+      for (const [key, approval] of this.pendingApprovals) {
+        if (approval.featureId === featureId) {
+          foundKey = key;
+          pending = approval;
+          break;
+        }
+      }
+    }
+
+    if (pending && foundKey) {
       logger.info(`Found and cancelling pending approval for feature ${featureId}`);
       // Wrapped reject clears timeout automatically
       pending.reject(new Error('Plan approval cancelled - feature was stopped'));
-      this.pendingApprovals.delete(featureId);
+      this.pendingApprovals.delete(foundKey);
     } else {
       logger.info(`No pending approval to cancel for feature ${featureId}`);
     }
   }
 
   /** Check if a feature has a pending plan approval. */
-  hasPendingApproval(featureId: string): boolean {
-    return this.pendingApprovals.has(featureId);
+  hasPendingApproval(featureId: string, projectPath?: string): boolean {
+    if (projectPath) {
+      return this.pendingApprovals.has(this.approvalKey(projectPath, featureId));
+    }
+    // Fallback: search by featureId (backward compatibility)
+    for (const approval of this.pendingApprovals.values()) {
+      if (approval.featureId === featureId) {
+        return true;
+      }
+    }
+    return false;
   }
 
   /** Get timeout from project settings or default (30 min). */

apps/server/tests/unit/services/feature-state-manager.test.ts

@@ -71,20 +71,21 @@ describe('FeatureStateManager', () => {
 
   describe('loadFeature', () => {
     it('should load feature from disk', async () => {
-      (secureFs.readFile as Mock).mockResolvedValue(JSON.stringify(mockFeature));
+      (readJsonWithRecovery as Mock).mockResolvedValue({ data: mockFeature, recovered: false });
 
       const feature = await manager.loadFeature('/project', 'feature-123');
 
       expect(feature).toEqual(mockFeature);
       expect(getFeatureDir).toHaveBeenCalledWith('/project', 'feature-123');
-      expect(secureFs.readFile).toHaveBeenCalledWith(
+      expect(readJsonWithRecovery).toHaveBeenCalledWith(
         '/project/.automaker/features/feature-123/feature.json',
-        'utf-8'
+        null,
+        expect.objectContaining({ autoRestore: true })
       );
     });
 
     it('should return null if feature does not exist', async () => {
-      (secureFs.readFile as Mock).mockRejectedValue(new Error('ENOENT'));
+      (readJsonWithRecovery as Mock).mockRejectedValue(new Error('ENOENT'));
 
       const feature = await manager.loadFeature('/project', 'non-existent');
 
@@ -92,7 +93,8 @@ describe('FeatureStateManager', () => {
     });
 
     it('should return null if feature JSON is invalid', async () => {
-      (secureFs.readFile as Mock).mockResolvedValue('not valid json');
+      // readJsonWithRecovery returns null as the default value when JSON is invalid
+      (readJsonWithRecovery as Mock).mockResolvedValue({ data: null, recovered: false });
 
       const feature = await manager.loadFeature('/project', 'feature-123');