fix(core,cli): address review findings for plan dir resolution and security

This commit addresses several critical findings from the review bot:

- **Security:** Implemented defense-in-depth symlink resolution. Removed insecure string-based fallbacks in `Storage.getPlansDir` and added a mandatory `isSubpath` validation AFTER directory creation in `Config.getPlansDir` to prevent TOCTOU traversal attacks.
- **Architecture:** Fixed a race condition where active extension context was mutated synchronously in `AppContainer`, potentially corrupting concurrent background tasks. Mutation now occurs within the command execution pipeline.
- **Robustness:** Switched to canonical path checking for `plan` command detection to support aliases and subcommands.
- **Regressions:** Added a `planEnabled` guard to prevent unwanted directory creation when the planning feature is disabled.
- **Validation:** Added exhaustive unit tests covering sequential context switching, shared directory deduplication, and symlink security edge cases.

packages/cli/src/ui/AppContainer.test.tsx

@@ -3183,159 +3183,6 @@ describe('AppContainer State Management', () => {
       unmount();
     });
 
-    it('sets activeExtensionContext when an extension command WITH a plan dir is executed', async () => {
-      const { checkPermissions } = await import(
-        './hooks/atCommandProcessor.js'
-      );
-      vi.mocked(checkPermissions).mockResolvedValue([]);
-
-      mockedUseSlashCommandProcessor.mockReturnValue({
-        handleSlashCommand: vi.fn(),
-        slashCommands: [
-          {
-            name: 'conductor:setup',
-            extensionName: 'conductor',
-            description: 'test',
-            action: vi.fn(),
-          },
-        ],
-        pendingHistoryItems: [],
-        commandContext: {},
-        shellConfirmationRequest: null,
-        confirmationRequest: null,
-      });
-
-      const spyHasExtensionPlanDir = vi
-        .spyOn(mockConfig, 'hasExtensionPlanDir')
-        .mockReturnValue(true);
-      const spySetActiveExtensionContext = vi.spyOn(
-        mockConfig,
-        'setActiveExtensionContext',
-      );
-
-      const { unmount } = await act(async () => renderAppContainer());
-
-      expect(capturedUIActions).toBeTruthy();
-
-      await act(async () =>
-        capturedUIActions.handleFinalSubmit('/conductor:setup'),
-      );
-
-      expect(spyHasExtensionPlanDir).toHaveBeenCalledWith('conductor');
-      expect(spySetActiveExtensionContext).toHaveBeenCalledWith('conductor');
-
-      unmount();
-    });
-
-    it('clears activeExtensionContext when an extension command WITHOUT a plan dir is executed', async () => {
-      const { checkPermissions } = await import(
-        './hooks/atCommandProcessor.js'
-      );
-      vi.mocked(checkPermissions).mockResolvedValue([]);
-
-      mockedUseSlashCommandProcessor.mockReturnValue({
-        handleSlashCommand: vi.fn(),
-        slashCommands: [
-          {
-            name: 'other:cmd',
-            extensionName: 'other',
-            description: 'test',
-            action: vi.fn(),
-          },
-        ],
-        pendingHistoryItems: [],
-        commandContext: {},
-        shellConfirmationRequest: null,
-        confirmationRequest: null,
-      });
-
-      const spyHasExtensionPlanDir = vi
-        .spyOn(mockConfig, 'hasExtensionPlanDir')
-        .mockReturnValue(false);
-      const spySetActiveExtensionContext = vi.spyOn(
-        mockConfig,
-        'setActiveExtensionContext',
-      );
-
-      const { unmount } = await act(async () => renderAppContainer());
-
-      expect(capturedUIActions).toBeTruthy();
-
-      await act(async () => capturedUIActions.handleFinalSubmit('/other:cmd'));
-
-      expect(spyHasExtensionPlanDir).toHaveBeenCalledWith('other');
-      expect(spySetActiveExtensionContext).toHaveBeenCalledWith(undefined);
-
-      unmount();
-    });
-
-    it('clears activeExtensionContext when /plan is explicitly executed', async () => {
-      const { checkPermissions } = await import(
-        './hooks/atCommandProcessor.js'
-      );
-      vi.mocked(checkPermissions).mockResolvedValue([]);
-
-      mockedUseSlashCommandProcessor.mockReturnValue({
-        handleSlashCommand: vi.fn(),
-        slashCommands: [{ name: 'plan', description: 'test', action: vi.fn() }],
-        pendingHistoryItems: [],
-        commandContext: {},
-        shellConfirmationRequest: null,
-        confirmationRequest: null,
-      });
-
-      const spySetActiveExtensionContext = vi.spyOn(
-        mockConfig,
-        'setActiveExtensionContext',
-      );
-
-      const { unmount } = await act(async () => renderAppContainer());
-
-      expect(capturedUIActions).toBeTruthy();
-
-      await act(async () =>
-        capturedUIActions.handleFinalSubmit('/plan my task'),
-      );
-
-      expect(spySetActiveExtensionContext).toHaveBeenCalledWith(undefined);
-
-      unmount();
-    });
-
-    it('does NOT clear activeExtensionContext when a standard non-plan command is executed', async () => {
-      const { checkPermissions } = await import(
-        './hooks/atCommandProcessor.js'
-      );
-      vi.mocked(checkPermissions).mockResolvedValue([]);
-
-      mockedUseSlashCommandProcessor.mockReturnValue({
-        handleSlashCommand: vi.fn(),
-        slashCommands: [{ name: 'help', description: 'test', action: vi.fn() }],
-        pendingHistoryItems: [],
-        commandContext: {},
-        shellConfirmationRequest: null,
-        confirmationRequest: null,
-      });
-
-      const spySetActiveExtensionContext = vi.spyOn(
-        mockConfig,
-        'setActiveExtensionContext',
-      );
-
-      const { unmount } = await act(async () => renderAppContainer());
-
-      expect(capturedUIActions).toBeTruthy();
-
-      await act(async () => capturedUIActions.handleFinalSubmit('/help'));
-
-      // It should not touch the context at all
-      expect(spySetActiveExtensionContext).not.toHaveBeenCalled();
-
-      unmount();
-    });
-  });
-
-  describe('Overflow Hint Handling', () => {
     beforeEach(() => {
       vi.useFakeTimers();
     });

packages/cli/src/ui/AppContainer.tsx

@@ -1313,18 +1313,6 @@ Logging in with Google... Restarting Gemini CLI to continue.
         slashCommands ?? [],
       );
 
-      if (config) {
-        if (parsedCommand.extensionContext) {
-          if (config.hasExtensionPlanDir(parsedCommand.extensionContext)) {
-            config.setActiveExtensionContext(parsedCommand.extensionContext);
-          } else {
-            config.setActiveExtensionContext(undefined);
-          }
-        } else if (parsedCommand.commandToExecute?.name === 'plan') {
-          config.setActiveExtensionContext(undefined);
-        }
-      }
-
       const isSlash = isSlashCommand(submittedValue.trim());
       const isIdle = streamingState === StreamingState.Idle;
       const isAgentRunning =

packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx

@@ -993,4 +993,155 @@ describe('useSlashCommandProcessor', () => {
       expect(result.current.slashCommands).toEqual([newCommand]),
     );
   });
+  describe('Active Extension Context Switching', () => {
+    it('sets active extension context when a command with a plan dir is executed', async () => {
+      const extensionCommand = createTestCommand({
+        name: 'conductor:setup',
+        extensionName: 'conductor',
+        action: vi.fn(),
+      });
+
+      const spyHasPlanDir = vi
+        .spyOn(mockConfig, 'hasExtensionPlanDir')
+        .mockReturnValue(true);
+      const spySetContext = vi.spyOn(mockConfig, 'setActiveExtensionContext');
+
+      const hook = await setupProcessorHook({
+        builtinCommands: [extensionCommand],
+      });
+
+      await waitFor(() => expect(hook.current.slashCommands!.length).toBe(1));
+
+      await act(async () => {
+        await hook.current.handleSlashCommand('/conductor:setup');
+      });
+
+      expect(spyHasPlanDir).toHaveBeenCalledWith('conductor');
+      expect(spySetContext).toHaveBeenCalledWith('conductor');
+    });
+
+    it('clears active extension context when a command WITHOUT a plan dir is executed', async () => {
+      const extensionCommand = createTestCommand({
+        name: 'other:cmd',
+        extensionName: 'other',
+        action: vi.fn(),
+      });
+
+      const spyHasPlanDir = vi
+        .spyOn(mockConfig, 'hasExtensionPlanDir')
+        .mockReturnValue(false);
+      const spySetContext = vi.spyOn(mockConfig, 'setActiveExtensionContext');
+
+      const hook = await setupProcessorHook({
+        builtinCommands: [extensionCommand],
+      });
+
+      await waitFor(() => expect(hook.current.slashCommands!.length).toBe(1));
+
+      await act(async () => {
+        await hook.current.handleSlashCommand('/other:cmd');
+      });
+
+      expect(spyHasPlanDir).toHaveBeenCalledWith('other');
+      expect(spySetContext).toHaveBeenCalledWith(undefined);
+    });
+
+    it('clears active extension context when the canonical /plan command is executed', async () => {
+      const planCommand = createTestCommand({
+        name: 'plan',
+        action: vi.fn(),
+      });
+
+      const spySetContext = vi.spyOn(mockConfig, 'setActiveExtensionContext');
+
+      const hook = await setupProcessorHook({
+        builtinCommands: [planCommand],
+      });
+
+      await waitFor(() => expect(hook.current.slashCommands!.length).toBe(1));
+
+      await act(async () => {
+        await hook.current.handleSlashCommand('/plan my task');
+      });
+
+      expect(spySetContext).toHaveBeenCalledWith(undefined);
+    });
+
+    it('clears active extension context when a /plan alias or subcommand is executed', async () => {
+      const planCommand = createTestCommand({
+        name: 'plan',
+        subCommands: [
+          createTestCommand({
+            name: 'create',
+          }),
+        ],
+        action: vi.fn(),
+      });
+
+      const spySetContext = vi.spyOn(mockConfig, 'setActiveExtensionContext');
+
+      const hook = await setupProcessorHook({
+        builtinCommands: [planCommand],
+      });
+
+      await waitFor(() => expect(hook.current.slashCommands!.length).toBe(1));
+
+      await act(async () => {
+        await hook.current.handleSlashCommand('/plan create');
+      });
+
+      expect(spySetContext).toHaveBeenCalledWith(undefined);
+    });
+
+    it('handles a sequence of context switches between extensions and default plan mode', async () => {
+      const extA = createTestCommand({
+        name: 'extA',
+        extensionName: 'extA',
+        action: vi.fn(),
+      });
+      const extB = createTestCommand({
+        name: 'extB',
+        extensionName: 'extB',
+        action: vi.fn(),
+      });
+      const planCmd = createTestCommand({
+        name: 'plan',
+        action: vi.fn(),
+      });
+
+      const spySetContext = vi.spyOn(mockConfig, 'setActiveExtensionContext');
+      vi.spyOn(mockConfig, 'hasExtensionPlanDir').mockReturnValue(true);
+
+      const hook = await setupProcessorHook({
+        builtinCommands: [extA, extB, planCmd],
+      });
+
+      await waitFor(() => expect(hook.current.slashCommands!.length).toBe(3));
+
+      // 1. Run Ext A
+      await act(async () => {
+        await hook.current.handleSlashCommand('/extA');
+      });
+      expect(spySetContext).toHaveBeenLastCalledWith('extA');
+
+      // 2. Run Ext B
+      await act(async () => {
+        await hook.current.handleSlashCommand('/extB');
+      });
+      expect(spySetContext).toHaveBeenLastCalledWith('extB');
+
+      // 3. Run /plan (Default)
+      await act(async () => {
+        await hook.current.handleSlashCommand('/plan my task');
+      });
+      expect(spySetContext).toHaveBeenLastCalledWith(undefined);
+
+      // 4. Run /clear (Global)
+      await act(async () => {
+        await hook.current.handleSlashCommand('/help');
+      });
+      // Context should still be undefined
+      expect(spySetContext).toHaveBeenLastCalledWith(undefined);
+    });
+  });
 });

packages/cli/src/ui/hooks/slashCommandProcessor.ts

@@ -368,8 +368,21 @@ export const useSlashCommandProcessor = (
         commandToExecute,
         args,
         canonicalPath: resolvedCommandPath,
+        extensionContext,
       } = parseSlashCommand(trimmed, commands);
 
+      if (config) {
+        if (extensionContext) {
+          if (config.hasExtensionPlanDir(extensionContext)) {
+            config.setActiveExtensionContext(extensionContext);
+          } else {
+            config.setActiveExtensionContext(undefined);
+          }
+        } else if (resolvedCommandPath?.[0] === 'plan') {
+          config.setActiveExtensionContext(undefined);
+        }
+      }
+
       // If the input doesn't match any known command, check if MCP servers
       // are still loading (the command might come from an MCP server).
       // Otherwise, treat it as regular text input (e.g. file paths like

packages/core/src/config/config.test.ts

@@ -3371,6 +3371,38 @@ describe('Plans Directory Initialization', () => {
     );
   });
 
+  it('should deduplicate and cache when multiple extensions (or default) use the same directory', async () => {
+    vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
+    const config = new Config({
+      ...baseParams,
+      plan: true,
+    });
+
+    await config.initialize();
+
+    // 1. Call for Default Plan Dir
+    const defaultDir = config.getPlansDir();
+    expect(fs.mkdirSync).toHaveBeenCalledTimes(1);
+
+    // 2. Mock an extension that happens to use the SAME directory string
+    vi.spyOn(
+      config as unknown as {
+        getActiveExtensionPlanDir: () => string | undefined;
+      },
+      'getActiveExtensionPlanDir',
+    ).mockReturnValue(
+      'plans', // This will resolve to the same path as the default in our mock setup
+    );
+
+    const extDir = config.getPlansDir();
+
+    // It should be the same path
+    expect(extDir).toBe(defaultDir);
+
+    // It should NOT have called mkdirSync a second time
+    expect(fs.mkdirSync).toHaveBeenCalledTimes(1);
+  });
+
   it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => {
     vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
     const config = new Config({