fix(core): resolve Plan Mode deadlock during plan file creation due to sandbox restrictions (#24047)

Author: DavidAPierce

evals/plan_mode.eval.ts

@@ -283,4 +283,47 @@ describe('plan_mode', () => {
       assertModelHasOutput(result);
     },
   });
+
+  evalTest('ALWAYS_PASSES', {
+    name: 'should transition from plan mode to normal execution and create a plan file from scratch',
+    params: {
+      settings,
+    },
+    prompt:
+      'Enter plan mode and plan to create a new module called foo. The plan should be saved as foo-plan.md. Then, exit plan mode.',
+    assert: async (rig, result) => {
+      const enterPlanCalled = await rig.waitForToolCall('enter_plan_mode');
+      expect(
+        enterPlanCalled,
+        'Expected enter_plan_mode tool to be called',
+      ).toBe(true);
+
+      const exitPlanCalled = await rig.waitForToolCall('exit_plan_mode');
+      expect(exitPlanCalled, 'Expected exit_plan_mode tool to be called').toBe(
+        true,
+      );
+
+      await rig.waitForTelemetryReady();
+      const toolLogs = rig.readToolLogs();
+
+      // Check if the plan file was written successfully
+      const planWrite = toolLogs.find(
+        (log) =>
+          log.toolRequest.name === 'write_file' &&
+          log.toolRequest.args.includes('foo-plan.md'),
+      );
+
+      expect(
+        planWrite,
+        'Expected write_file to be called for foo-plan.md',
+      ).toBeDefined();
+
+      expect(
+        planWrite?.toolRequest.success,
+        `Expected write_file to succeed, but got error: ${planWrite?.toolRequest.error}`,
+      ).toBe(true);
+
+      assertModelHasOutput(result);
+    },
+  });
 });

packages/core/src/config/config.ts

@@ -979,39 +979,33 @@ export class Config implements McpContext, AgentLoopContext {
           networkAccess: false,
         };
 
-    this._sandboxManager = createSandboxManager(this.sandbox, {
-      workspace: params.targetDir,
-    });
-
-    if (
-      !(this._sandboxManager instanceof NoopSandboxManager) &&
-      this.sandbox.enabled
-    ) {
-      this.fileSystemService = new SandboxedFileSystemService(
-        this._sandboxManager,
-        params.targetDir,
-      );
-    } else {
-      this.fileSystemService = new StandardFileSystemService();
-    }
+    this.targetDir = path.resolve(params.targetDir);
+    this.folderTrust = params.folderTrust ?? false;
+    this.workspaceContext = new WorkspaceContext(this.targetDir, []);
+    this.pendingIncludeDirectories = params.includeDirectories ?? [];
+    this.debugMode = params.debugMode;
+    this.question = params.question;
+    this.worktreeSettings = params.worktreeSettings;
 
     this._sandboxPolicyManager = new SandboxPolicyManager();
     const initialApprovalMode =
       params.approvalMode ??
       params.policyEngineConfig?.approvalMode ??
       'default';
+
     this._sandboxManager = createSandboxManager(
       this.sandbox,
       {
-        workspace: params.targetDir,
+        workspace: this.targetDir,
+        includeDirectories: this.pendingIncludeDirectories,
         policyManager: this._sandboxPolicyManager,
       },
       initialApprovalMode,
     );
 
     if (
       !(this._sandboxManager instanceof NoopSandboxManager) &&
-      this.sandbox?.enabled
+      this.sandbox.enabled
     ) {
       this.fileSystemService = new SandboxedFileSystemService(
         this._sandboxManager,
@@ -1021,10 +1015,6 @@ export class Config implements McpContext, AgentLoopContext {
       this.fileSystemService = new StandardFileSystemService();
     }
 
-    this.targetDir = path.resolve(params.targetDir);
-    this.folderTrust = params.folderTrust ?? false;
-    this.workspaceContext = new WorkspaceContext(this.targetDir, []);
-    this.pendingIncludeDirectories = params.includeDirectories ?? [];
     this.debugMode = params.debugMode;
     this.question = params.question;
     this.worktreeSettings = params.worktreeSettings;

packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts

@@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { LinuxSandboxManager } from './LinuxSandboxManager.js';
 import type { SandboxRequest } from '../../services/sandboxManager.js';
 import fs from 'node:fs';
+import path from 'node:path';
 import * as shellUtils from '../../utils/shell-utils.js';
 
 vi.mock('node:fs', async () => {
@@ -350,6 +351,61 @@ describe('LinuxSandboxManager', () => {
         const binds = bwrapArgs.filter((a) => a === workspace);
         expect(binds.length).toBe(2);
       });
+
+      it('should bind the parent directory of a non-existent path', async () => {
+        vi.mocked(fs.existsSync).mockImplementation((p) => {
+          if (p === '/home/user/workspace/new-file.txt') return false;
+          return true;
+        });
+
+        const bwrapArgs = await getBwrapArgs({
+          command: '__write',
+          args: ['/home/user/workspace/new-file.txt'],
+          cwd: workspace,
+          env: {},
+          policy: {
+            allowedPaths: ['/home/user/workspace/new-file.txt'],
+          },
+        });
+
+        const parentDir = '/home/user/workspace';
+        const bindIndex = bwrapArgs.lastIndexOf(parentDir);
+        expect(bindIndex).not.toBe(-1);
+        expect(bwrapArgs[bindIndex - 2]).toBe('--bind-try');
+      });
+    });
+
+    describe('virtual commands', () => {
+      it('should translate __read to cat', async () => {
+        const testFile = path.join(workspace, 'file.txt');
+        const bwrapArgs = await getBwrapArgs({
+          command: '__read',
+          args: [testFile],
+          cwd: workspace,
+          env: {},
+        });
+
+        // args are: [...bwrapBaseArgs, '--', '/bin/cat', '.../file.txt']
+        expect(bwrapArgs[bwrapArgs.length - 2]).toBe('/bin/cat');
+        expect(bwrapArgs[bwrapArgs.length - 1]).toBe(testFile);
+      });
+
+      it('should translate __write to sh -c cat', async () => {
+        const testFile = path.join(workspace, 'file.txt');
+        const bwrapArgs = await getBwrapArgs({
+          command: '__write',
+          args: [testFile],
+          cwd: workspace,
+          env: {},
+        });
+
+        // args are: [...bwrapBaseArgs, '--', '/bin/sh', '-c', 'tee -- "$@" > /dev/null', '_', '.../file.txt']
+        expect(bwrapArgs[bwrapArgs.length - 5]).toBe('/bin/sh');
+        expect(bwrapArgs[bwrapArgs.length - 4]).toBe('-c');
+        expect(bwrapArgs[bwrapArgs.length - 3]).toBe('tee -- "$@" > /dev/null');
+        expect(bwrapArgs[bwrapArgs.length - 2]).toBe('_');
+        expect(bwrapArgs[bwrapArgs.length - 1]).toBe(testFile);
+      });
     });
 
     describe('forbiddenPaths', () => {

packages/core/src/sandbox/linux/LinuxSandboxManager.ts

@@ -182,9 +182,23 @@ export class LinuxSandboxManager implements SandboxManager {
 
     verifySandboxOverrides(allowOverrides, req.policy);
 
-    const commandName = await getCommandName(req);
+    let command = req.command;
+    let args = req.args;
+
+    // Translate virtual commands for sandboxed file system access
+    if (command === '__read') {
+      command = 'cat';
+    } else if (command === '__write') {
+      command = 'sh';
+      args = ['-c', 'cat > "$1"', '_', ...args];
+    }
+
+    const commandName = await getCommandName({ ...req, command, args });
     const isApproved = allowOverrides
-      ? await isStrictlyApproved(req, this.options.modeConfig?.approvedTools)
+      ? await isStrictlyApproved(
+          { ...req, command, args },
+          this.options.modeConfig?.approvedTools,
+        )
       : false;
     const workspaceWrite = !isReadonlyMode || isApproved;
     const networkAccess =
@@ -280,11 +294,36 @@ export class LinuxSandboxManager implements SandboxManager {
       bwrapArgs.push(bindFlag, mainGitDir, mainGitDir);
     }
 
+    const includeDirs = sanitizePaths(this.options.includeDirectories) || [];
+    for (const includeDir of includeDirs) {
+      try {
+        const resolved = tryRealpath(includeDir);
+        bwrapArgs.push('--ro-bind-try', resolved, resolved);
+      } catch {
+        // Ignore
+      }
+    }
+
     const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || [];
+
     const normalizedWorkspace = normalize(workspacePath).replace(/\/$/, '');
     for (const allowedPath of allowedPaths) {
       const resolved = tryRealpath(allowedPath);
-      if (!fs.existsSync(resolved)) continue;
+      if (!fs.existsSync(resolved)) {
+        // If the path doesn't exist, we still want to allow access to its parent
+        // if it's explicitly allowed, to enable creating it.
+        try {
+          const resolvedParent = tryRealpath(dirname(resolved));
+          bwrapArgs.push(
+            req.command === '__write' ? '--bind-try' : bindFlag,
+            resolvedParent,
+            resolvedParent,
+          );
+        } catch {
+          // Ignore
+        }
+        continue;
+      }
       const normalizedAllowedPath = normalize(resolved).replace(/\/$/, '');
       if (normalizedAllowedPath !== normalizedWorkspace) {
         bwrapArgs.push('--bind-try', resolved, resolved);