refactor: streamline non-interactive session handling by removing settings parameter and introducing minimal settings instance

2025-12-19 09:33:53 +00:00 · 2025-11-18 12:20:47 +08:00
parent f9a2b7b032
commit 13cc8f9f55
6 changed files with 67 additions and 66 deletions
--- a/docs/features/subagents.md
+++ b/docs/features/subagents.md
@@ -106,7 +106,7 @@ Subagents are configured using Markdown files with YAML frontmatter. This format
 ---
 name: agent-name
 description: Brief description of when and how to use this agent
-tools: 
+tools:
  - tool1
  - tool2
  - tool3 # Optional
@@ -170,7 +170,7 @@ Perfect for comprehensive test creation and test-driven development.
 ---
 name: testing-expert
 description: Writes comprehensive unit tests, integration tests, and handles test automation with best practices
-tools: 
+tools:
  - read_file
  - write_file
  - read_many_files
@@ -214,7 +214,7 @@ Specialized in creating clear, comprehensive documentation.
 ---
 name: documentation-writer
 description: Creates comprehensive documentation, README files, API docs, and user guides
-tools: 
+tools:
  - read_file
  - write_file
  - read_many_files
@@ -267,7 +267,7 @@ Focused on code quality, security, and best practices.
 ---
 name: code-reviewer
 description: Reviews code for best practices, security issues, performance, and maintainability
-tools: 
+tools:
  - read_file
  - read_many_files
 ---
@@ -311,7 +311,7 @@ Optimized for React development, hooks, and component patterns.
 ---
 name: react-specialist
 description: Expert in React development, hooks, component patterns, and modern React best practices
-tools: 
+tools:
  - read_file
  - write_file
  - read_many_files
--- a/packages/cli/src/config/settings.ts
+++ b/packages/cli/src/config/settings.ts
@@ -484,6 +484,27 @@ export class LoadedSettings {
  }
 }

+/**
+ * Creates a minimal LoadedSettings instance with empty settings.
+ * Used in stream-json mode where settings are ignored.
+ */
+export function createMinimalSettings(): LoadedSettings {
+  const emptySettingsFile: SettingsFile = {
+    path: '',
+    settings: {},
+    originalSettings: {},
+    rawJson: '{}',
+  };
+  return new LoadedSettings(
+    emptySettingsFile,
+    emptySettingsFile,
+    emptySettingsFile,
+    emptySettingsFile,
+    false,
+    new Set(),
+  );
+}
+
 function findEnvFile(startDir: string): string | null {
  let currentDir = path.resolve(startDir);
  while (true) {
--- a/packages/cli/src/gemini.test.tsx
+++ b/packages/cli/src/gemini.test.tsx
@@ -357,12 +357,9 @@ describe('gemini.tsx main function', () => {
    }

    expect(runStreamJsonSpy).toHaveBeenCalledTimes(1);
-    const [configArg, settingsArg, promptArg] = runStreamJsonSpy.mock.calls[0];
+    const [configArg, inputArg] = runStreamJsonSpy.mock.calls[0];
    expect(configArg).toBe(validatedConfig);
-    expect(settingsArg).toMatchObject({
-      merged: expect.objectContaining({ security: expect.any(Object) }),
-    });
-    expect(promptArg).toBe('hello stream');
+    expect(inputArg).toBe('hello stream');

    expect(validateAuthSpy).toHaveBeenCalledWith(
      undefined,
--- a/packages/cli/src/gemini.tsx
+++ b/packages/cli/src/gemini.tsx
@@ -446,9 +446,7 @@ export async function main() {

      await runNonInteractiveStreamJson(
        nonInteractiveConfig,
-        settings,
        trimmedInput.length > 0 ? trimmedInput : '',
-        prompt_id,
      );
      await runExitCleanup();
      process.exit(0);
--- a/packages/cli/src/nonInteractive/session.test.ts
+++ b/packages/cli/src/nonInteractive/session.test.ts
@@ -6,7 +6,6 @@

 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import type { Config } from '@qwen-code/qwen-code-core';
-import type { LoadedSettings } from '../config/settings.js';
 import { runNonInteractiveStreamJson } from './session.js';
 import type {
  CLIUserMessage,
@@ -74,14 +73,6 @@ function createConfig(overrides: ConfigOverrides = {}): Config {
  return { ...base, ...overrides } as unknown as Config;
 }

-function createSettings(): LoadedSettings {
-  return {
-    merged: {
-      security: { auth: {} },
-    },
-  } as unknown as LoadedSettings;
-}
-
 function createUserMessage(content: string): CLIUserMessage {
  return {
    type: 'user',
@@ -145,7 +136,6 @@ function createControlCancel(requestId: string): ControlCancelRequest {

 describe('runNonInteractiveStreamJson', () => {
  let config: Config;
-  let settings: LoadedSettings;
  let mockInputReader: {
    read: () => AsyncGenerator<
      | CLIUserMessage
@@ -170,7 +160,6 @@ describe('runNonInteractiveStreamJson', () => {

  beforeEach(() => {
    config = createConfig();
-    settings = createSettings();
    runNonInteractiveMock.mockReset();

    // Setup mocks
@@ -232,7 +221,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield initRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockConsolePatcher.patch).toHaveBeenCalledTimes(1);
    expect(mockDispatcher.dispatch).toHaveBeenCalledWith(initRequest);
@@ -246,7 +235,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).toHaveBeenCalledTimes(1);
    const runCall = runNonInteractiveMock.mock.calls[0];
@@ -272,7 +261,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage2;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).toHaveBeenCalledTimes(2);
  });
@@ -293,7 +282,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage2;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    // Both messages should be processed
    expect(runNonInteractiveMock).toHaveBeenCalledTimes(2);
@@ -308,7 +297,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield controlRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.dispatch).toHaveBeenCalledTimes(2);
    expect(mockDispatcher.dispatch).toHaveBeenNthCalledWith(1, initRequest);
@@ -324,7 +313,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield controlResponse;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.handleControlResponse).toHaveBeenCalledWith(
      controlResponse,
@@ -340,7 +329,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield cancelRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.handleCancel).toHaveBeenCalledWith('req-2');
  });
@@ -360,7 +349,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield controlRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.dispatch).toHaveBeenCalledWith(controlRequest);
  });
@@ -380,7 +369,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield controlResponse;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.handleControlResponse).toHaveBeenCalledWith(
      controlResponse,
@@ -394,12 +383,12 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).toHaveBeenCalledTimes(1);
    expect(runNonInteractiveMock).toHaveBeenCalledWith(
      config,
-      settings,
+      expect.objectContaining({ merged: expect.any(Object) }),
      'Test message',
      expect.stringContaining('test-session'),
      expect.objectContaining({
@@ -427,12 +416,12 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).toHaveBeenCalledTimes(1);
    expect(runNonInteractiveMock).toHaveBeenCalledWith(
      config,
-      settings,
+      expect.objectContaining({ merged: expect.any(Object) }),
      'First part\nSecond part',
      expect.stringContaining('test-session'),
      expect.objectContaining({
@@ -457,7 +446,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).not.toHaveBeenCalled();
  });
@@ -472,7 +461,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    // Error should be caught and handled gracefully
  });
@@ -484,9 +473,9 @@ describe('runNonInteractiveStreamJson', () => {
      throw streamError;
    } as typeof mockInputReader.read;

-    await expect(
-      runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id'),
-    ).rejects.toThrow('Stream error');
+    await expect(runNonInteractiveStreamJson(config, '')).rejects.toThrow(
+      'Stream error',
+    );

    expect(mockConsolePatcher.cleanup).toHaveBeenCalled();
  });
@@ -517,7 +506,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    // Verify initialization happened
    expect(mockDispatcher.dispatch).toHaveBeenCalledWith(initRequest);
@@ -536,7 +525,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield userMessage2;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(runNonInteractiveMock).toHaveBeenCalledTimes(2);
    const promptId1 = runNonInteractiveMock.mock.calls[0][3] as string;
@@ -553,7 +542,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield controlRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    // Should not transition to idle since it's not an initialize request
    expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
@@ -564,7 +553,7 @@ describe('runNonInteractiveStreamJson', () => {
      // Empty stream - should complete immediately
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockConsolePatcher.patch).toHaveBeenCalledTimes(1);
    expect(mockConsolePatcher.cleanup).toHaveBeenCalledTimes(1);
@@ -575,7 +564,7 @@ describe('runNonInteractiveStreamJson', () => {
      // Empty stream
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');
  });

  it('calls dispatcher shutdown on completion', async () => {
@@ -585,7 +574,7 @@ describe('runNonInteractiveStreamJson', () => {
      yield initRequest;
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockDispatcher.shutdown).toHaveBeenCalledTimes(1);
  });
@@ -595,7 +584,7 @@ describe('runNonInteractiveStreamJson', () => {
      // Empty stream
    };

-    await runNonInteractiveStreamJson(config, settings, '', 'test-prompt-id');
+    await runNonInteractiveStreamJson(config, '');

    expect(mockConsolePatcher.cleanup).toHaveBeenCalled();
  });
--- a/packages/cli/src/nonInteractive/session.ts
+++ b/packages/cli/src/nonInteractive/session.ts
@@ -38,7 +38,7 @@ import {
  isControlResponse,
  isControlCancel,
 } from './types.js';
-import type { LoadedSettings } from '../config/settings.js';
+import { createMinimalSettings } from '../config/settings.js';
 import { runNonInteractive } from '../nonInteractiveCli.js';
 import { ConsolePatcher } from '../ui/utils/ConsolePatcher.js';

@@ -87,7 +87,6 @@ class SessionManager {
  private userMessageQueue: CLIUserMessage[] = [];
  private abortController: AbortController;
  private config: Config;
-  private settings: LoadedSettings;
  private sessionId: string;
  private promptIdCounter: number = 0;
  private inputReader: StreamJsonInputReader;
@@ -100,13 +99,8 @@ class SessionManager {
  private shutdownHandler: (() => void) | null = null;
  private initialPrompt: CLIUserMessage | null = null;

-  constructor(
-    config: Config,
-    settings: LoadedSettings,
-    initialPrompt?: CLIUserMessage,
-  ) {
+  constructor(config: Config, initialPrompt?: CLIUserMessage) {
    this.config = config;
-    this.settings = settings;
    this.sessionId = config.getSessionId();
    this.debugMode = config.getDebugMode();
    this.abortController = new AbortController();
@@ -569,11 +563,17 @@ class SessionManager {
    const promptId = this.getNextPromptId();

    try {
-      await runNonInteractive(this.config, this.settings, input, promptId, {
-        abortController: this.abortController,
-        adapter: this.outputAdapter,
-        controlService: this.controlService ?? undefined,
-      });
+      await runNonInteractive(
+        this.config,
+        createMinimalSettings(),
+        input,
+        promptId,
+        {
+          abortController: this.abortController,
+          adapter: this.outputAdapter,
+          controlService: this.controlService ?? undefined,
+        },
+      );
    } catch (error) {
      // Error already handled by runNonInteractive via adapter.emitResult
      if (this.debugMode) {
@@ -686,15 +686,11 @@ function extractUserMessageText(message: CLIUserMessage): string | null {
 * Entry point for stream-json mode
 *
 * @param config - Configuration object
- * @param settings - Loaded settings
 * @param input - Optional initial prompt input to process before reading from stream
- * @param promptId - Prompt ID (not used in stream-json mode but kept for API compatibility)
 */
 export async function runNonInteractiveStreamJson(
  config: Config,
-  settings: LoadedSettings,
  input: string,
-  _promptId: string,
 ): Promise<void> {
  const consolePatcher = new ConsolePatcher({
    debugMode: config.getDebugMode(),
@@ -717,7 +713,7 @@ export async function runNonInteractiveStreamJson(
      };
    }

-    const manager = new SessionManager(config, settings, initialPrompt);
+    const manager = new SessionManager(config, initialPrompt);
    await manager.run();
  } finally {
    consolePatcher.cleanup();