fix(core): refactor exit-plan-mode to use resolveAndValidatePlanPath consistently

This fixes unit test failures by centralizing path resolution logic.

Author: mahimashanware

packages/core/src/tools/exit-plan-mode.test.ts

@@ -72,8 +72,10 @@ describe('ExitPlanModeTool', () => {
 
   const createPlanFile = (name: string, content: string) => {
     const filePath = path.join(mockPlansDir, name);
+    // Ensure parent directory exists for nested tests
+    fs.mkdirSync(path.dirname(filePath), { recursive: true });
     fs.writeFileSync(filePath, content);
-    return path.join('plans', name);
+    return name;
   };
 
   describe('shouldConfirmExecute', () => {

packages/core/src/tools/exit-plan-mode.ts

@@ -19,9 +19,13 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
 import path from 'node:path';
 import type { Config } from '../config/config.js';
 import { EXIT_PLAN_MODE_TOOL_NAME } from './tool-names.js';
-import { validatePlanPath, validatePlanContent } from '../utils/planUtils.js';
+import {
+  validatePlanPath,
+  validatePlanContent,
+  resolveAndValidatePlanPath,
+} from '../utils/planUtils.js';
 import { ApprovalMode } from '../policy/types.js';
-import { resolveToRealPath, isSubpath } from '../utils/paths.js';
+// Remove unused imports
 import { logPlanExecution } from '../telemetry/loggers.js';
 import { PlanExecutionEvent } from '../telemetry/types.js';
 import { getExitPlanModeDefinition } from './definitions/coreTools.js';
@@ -59,18 +63,19 @@ export class ExitPlanModeTool extends BaseDeclarativeTool<
     if (!params.plan_filename || params.plan_filename.trim() === '') {
       return 'plan_filename is required.';
     }
-
-    const safeFilename = path.basename(params.plan_filename);
-    const plansDir = resolveToRealPath(this.config.storage.getPlansDir());
-    const resolvedPath = path.join(
-      this.config.storage.getPlansDir(),
-      safeFilename,
-    );
-
-    const realPath = resolveToRealPath(resolvedPath);
-
-    if (!isSubpath(plansDir, realPath)) {
-      return `Access denied: plan path (${resolvedPath}) must be within the designated plans directory (${plansDir}).`;
+    try {
+      resolveAndValidatePlanPath(
+        params.plan_filename,
+        this.config.getPlansDir(),
+      );
+    } catch (e) {
+      if (e instanceof Error && e.message.startsWith('Security violation')) {
+        return `Access denied: plan path (${path.join(
+          this.config.getPlansDir(),
+          params.plan_filename,
+        )}) must be within the designated plans directory (${this.config.getPlansDir()}).`;
+      }
+      return e instanceof Error ? e.message : String(e);
     }
 
     return null;
@@ -121,7 +126,7 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
 
     const pathError = await validatePlanPath(
       this.params.plan_filename,
-      this.config.storage.getPlansDir(),
+      this.config.getPlansDir(),
     );
     if (pathError) {
       this.planValidationError = pathError;
@@ -171,16 +176,18 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
   }
 
   getDescription(): string {
-    return `Requesting plan approval for: ${path.join(this.config.storage.getPlansDir(), this.params.plan_filename)}`;
+    return `Requesting plan approval for: ${path.join(this.config.getPlansDir(), this.params.plan_filename)}`;
   }
 
   /**
    * Returns the resolved plan path.
    * Note: Validation is done in validateToolParamValues, so this assumes the path is valid.
    */
   private getResolvedPlanPath(): string {
-    const safeFilename = path.basename(this.params.plan_filename);
-    return path.join(this.config.storage.getPlansDir(), safeFilename);
+    return resolveAndValidatePlanPath(
+      this.params.plan_filename,
+      this.config.getPlansDir(),
+    );
   }
 
   async execute({ abortSignal: _signal }: ExecuteOptions): Promise<ToolResult> {