From 13cc8f9f558311260a5dda223f43ef2f04e1d1e4 Mon Sep 17 00:00:00 2001 From: "mingholy.lmh" Date: Tue, 18 Nov 2025 12:20:47 +0800 Subject: [PATCH] refactor: streamline non-interactive session handling by removing settings parameter and introducing minimal settings instance --- docs/features/subagents.md | 10 +-- packages/cli/src/config/settings.ts | 21 +++++++ packages/cli/src/gemini.test.tsx | 7 +-- packages/cli/src/gemini.tsx | 2 - .../cli/src/nonInteractive/session.test.ts | 61 ++++++++----------- packages/cli/src/nonInteractive/session.ts | 32 +++++----- 6 files changed, 67 insertions(+), 66 deletions(-) diff --git a/docs/features/subagents.md b/docs/features/subagents.md index 225384ca..506d856f 100644 --- 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 diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index edc7709f..366600a7 100644 --- 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) { diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 423e8deb..d928be0d 100644 --- 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, diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 3b87f07e..002f34c1 100644 --- 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); diff --git a/packages/cli/src/nonInteractive/session.test.ts b/packages/cli/src/nonInteractive/session.test.ts index 20001c3a..61643fb3 100644 --- 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(); }); diff --git a/packages/cli/src/nonInteractive/session.ts b/packages/cli/src/nonInteractive/session.ts index a8d6f878..614208b7 100644 --- 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 { 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();