diff --git a/.gitignore b/.gitignore index 2484d9e8..2c3156b9 100644 --- a/.gitignore +++ b/.gitignore @@ -57,4 +57,3 @@ gha-creds-*.json # Log files patch_output.log -QWEN.md diff --git a/.vscode/launch.json b/.vscode/launch.json index 1966371c..d98757fb 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -73,7 +73,16 @@ "request": "launch", "name": "Launch CLI Non-Interactive", "runtimeExecutable": "npm", - "runtimeArgs": ["run", "start", "--", "-p", "${input:prompt}", "-y"], + "runtimeArgs": [ + "run", + "start", + "--", + "-p", + "${input:prompt}", + "-y", + "--output-format", + "stream-json" + ], "skipFiles": ["/**"], "cwd": "${workspaceFolder}", "console": "integratedTerminal", diff --git a/docs/cli/_meta.ts b/docs/cli/_meta.ts index 239757f1..1557b595 100644 --- a/docs/cli/_meta.ts +++ b/docs/cli/_meta.ts @@ -5,7 +5,6 @@ export default { commands: 'Commands', configuration: 'Configuration', 'configuration-v1': 'Configuration (v1)', - 'structured-output': 'Structured Output', themes: 'Themes', tutorials: 'Tutorials', 'keyboard-shortcuts': 'Keyboard Shortcuts', diff --git a/integration-tests/json-output.test.ts b/integration-tests/json-output.test.ts index 9c6bce18..8221aa5b 100644 --- a/integration-tests/json-output.test.ts +++ b/integration-tests/json-output.test.ts @@ -74,35 +74,236 @@ describe('JSON output', () => { expect(thrown).toBeDefined(); const message = (thrown as Error).message; - // The error JSON is written to stderr, so it should be in the error message - // Use a regex to find the first complete JSON object in the string - const jsonMatch = message.match(/{[\s\S]*}/); - - // Fail if no JSON-like text was found + // The error JSON is written to stdout as a CLIResultMessageError + // Extract stdout from the error message + const stdoutMatch = message.match(/Stdout:\n([\s\S]*?)(?:\n\nStderr:|$)/); expect( - jsonMatch, - 'Expected to find a JSON object in the error output', + stdoutMatch, + 'Expected to find stdout in the error message', ).toBeTruthy(); - let payload; + const stdout = stdoutMatch![1]; + let parsed: unknown[]; try { - // Parse the matched JSON string - payload = JSON.parse(jsonMatch![0]); + // Parse the JSON array from stdout + parsed = JSON.parse(stdout); } catch (parseError) { - console.error('Failed to parse the following JSON:', jsonMatch![0]); + console.error('Failed to parse the following JSON:', stdout); throw new Error( - `Test failed: Could not parse JSON from error message. Details: ${parseError}`, + `Test failed: Could not parse JSON from stdout. Details: ${parseError}`, ); } - // The JsonFormatter.formatError() outputs: { error: { type, message, code } } - expect(payload).toHaveProperty('error'); - expect(payload.error).toBeDefined(); - expect(payload.error.type).toBe('Error'); - expect(payload.error.code).toBe(1); - expect(payload.error.message).toContain( + // The output should be an array of messages + expect(Array.isArray(parsed)).toBe(true); + expect(parsed.length).toBeGreaterThan(0); + + // Find the result message with error + const resultMessage = parsed.find( + (msg: unknown) => + typeof msg === 'object' && + msg !== null && + 'type' in msg && + msg.type === 'result' && + 'is_error' in msg && + msg.is_error === true, + ) as { + type: string; + is_error: boolean; + subtype: string; + error?: { message: string; type?: string }; + }; + + expect(resultMessage).toBeDefined(); + expect(resultMessage.is_error).toBe(true); + expect(resultMessage).toHaveProperty('subtype'); + expect(resultMessage.subtype).toBe('error_during_execution'); + expect(resultMessage).toHaveProperty('error'); + expect(resultMessage.error).toBeDefined(); + expect(resultMessage.error?.message).toContain( 'configured auth type is qwen-oauth', ); - expect(payload.error.message).toContain('current auth type is openai'); + expect(resultMessage.error?.message).toContain( + 'current auth type is openai', + ); + }); + + it('should return line-delimited JSON messages for stream-json output format', async () => { + const result = await rig.run( + 'What is the capital of France?', + '--output-format', + 'stream-json', + ); + + // Stream-json output is line-delimited JSON (one JSON object per line) + const lines = result + .trim() + .split('\n') + .filter((line) => line.trim()); + expect(lines.length).toBeGreaterThan(0); + + // Parse each line as a JSON object + const messages: unknown[] = []; + for (const line of lines) { + try { + const parsed = JSON.parse(line); + messages.push(parsed); + } catch (parseError) { + throw new Error( + `Failed to parse JSON line: ${line}. Error: ${parseError}`, + ); + } + } + + // Should have at least system, assistant, and result messages + expect(messages.length).toBeGreaterThanOrEqual(3); + + // Find system message + const systemMessage = messages.find( + (msg: unknown) => + typeof msg === 'object' && + msg !== null && + 'type' in msg && + msg.type === 'system', + ); + expect(systemMessage).toBeDefined(); + expect(systemMessage).toHaveProperty('subtype'); + expect(systemMessage).toHaveProperty('session_id'); + + // Find assistant message + const assistantMessage = messages.find( + (msg: unknown) => + typeof msg === 'object' && + msg !== null && + 'type' in msg && + msg.type === 'assistant', + ); + expect(assistantMessage).toBeDefined(); + expect(assistantMessage).toHaveProperty('message'); + expect(assistantMessage).toHaveProperty('session_id'); + + // Find result message (should be the last message) + const resultMessage = messages[messages.length - 1] as { + type: string; + is_error: boolean; + result: string; + }; + expect(resultMessage).toBeDefined(); + expect( + typeof resultMessage === 'object' && + resultMessage !== null && + 'type' in resultMessage && + resultMessage.type === 'result', + ).toBe(true); + expect(resultMessage).toHaveProperty('is_error'); + expect(resultMessage.is_error).toBe(false); + expect(resultMessage).toHaveProperty('result'); + expect(typeof resultMessage.result).toBe('string'); + expect(resultMessage.result.toLowerCase()).toContain('paris'); + }); + + it('should include stream events when using stream-json with include-partial-messages', async () => { + const result = await rig.run( + 'What is the capital of France?', + '--output-format', + 'stream-json', + '--include-partial-messages', + ); + + // Stream-json output is line-delimited JSON (one JSON object per line) + const lines = result + .trim() + .split('\n') + .filter((line) => line.trim()); + expect(lines.length).toBeGreaterThan(0); + + // Parse each line as a JSON object + const messages: unknown[] = []; + for (const line of lines) { + try { + const parsed = JSON.parse(line); + messages.push(parsed); + } catch (parseError) { + throw new Error( + `Failed to parse JSON line: ${line}. Error: ${parseError}`, + ); + } + } + + // Should have more messages than without include-partial-messages + // because we're including stream events + expect(messages.length).toBeGreaterThan(3); + + // Find stream_event messages + const streamEvents = messages.filter( + (msg: unknown) => + typeof msg === 'object' && + msg !== null && + 'type' in msg && + msg.type === 'stream_event', + ); + expect(streamEvents.length).toBeGreaterThan(0); + + // Verify stream event structure + const firstStreamEvent = streamEvents[0]; + expect(firstStreamEvent).toHaveProperty('event'); + expect(firstStreamEvent).toHaveProperty('session_id'); + expect(firstStreamEvent).toHaveProperty('uuid'); + + // Check for expected stream event types + const eventTypes = streamEvents.map((event: unknown) => + typeof event === 'object' && + event !== null && + 'event' in event && + typeof event.event === 'object' && + event.event !== null && + 'type' in event.event + ? event.event.type + : null, + ); + + // Should have message_start event + expect(eventTypes).toContain('message_start'); + + // Should have content_block_start event + expect(eventTypes).toContain('content_block_start'); + + // Should have content_block_delta events + expect(eventTypes).toContain('content_block_delta'); + + // Should have content_block_stop event + expect(eventTypes).toContain('content_block_stop'); + + // Should have message_stop event + expect(eventTypes).toContain('message_stop'); + + // Verify that we still have the complete assistant message + const assistantMessage = messages.find( + (msg: unknown) => + typeof msg === 'object' && + msg !== null && + 'type' in msg && + msg.type === 'assistant', + ); + expect(assistantMessage).toBeDefined(); + expect(assistantMessage).toHaveProperty('message'); + + // Verify that we still have the result message + const resultMessage = messages[messages.length - 1] as { + type: string; + is_error: boolean; + result: string; + }; + expect(resultMessage).toBeDefined(); + expect( + typeof resultMessage === 'object' && + resultMessage !== null && + 'type' in resultMessage && + resultMessage.type === 'result', + ).toBe(true); + expect(resultMessage).toHaveProperty('is_error'); + expect(resultMessage.is_error).toBe(false); + expect(resultMessage).toHaveProperty('result'); + expect(resultMessage.result.toLowerCase()).toContain('paris'); }); }); diff --git a/integration-tests/test-helper.ts b/integration-tests/test-helper.ts index a1eb15c8..0fe658c5 100644 --- a/integration-tests/test-helper.ts +++ b/integration-tests/test-helper.ts @@ -340,7 +340,8 @@ export class TestRig { // as it would corrupt the JSON const isJsonOutput = commandArgs.includes('--output-format') && - commandArgs.includes('json'); + (commandArgs.includes('json') || + commandArgs.includes('stream-json')); // If we have stderr output and it's not a JSON test, include that also if (stderr && !isJsonOutput) { @@ -349,7 +350,23 @@ export class TestRig { resolve(result); } else { - reject(new Error(`Process exited with code ${code}:\n${stderr}`)); + // Check if this is a JSON output test - for JSON errors, the error is in stdout + const isJsonOutputOnError = + commandArgs.includes('--output-format') && + (commandArgs.includes('json') || + commandArgs.includes('stream-json')); + + // For JSON output tests, include stdout in the error message + // as the error JSON is written to stdout + if (isJsonOutputOnError && stdout) { + reject( + new Error( + `Process exited with code ${code}:\nStdout:\n${stdout}\n\nStderr:\n${stderr}`, + ), + ); + } else { + reject(new Error(`Process exited with code ${code}:\n${stderr}`)); + } } }); }); 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 2ff352ee..002f34c1 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -220,12 +220,6 @@ export async function main() { } const isDebugMode = cliConfig.isDebugMode(argv); - const consolePatcher = new ConsolePatcher({ - stderr: true, - debugMode: isDebugMode, - }); - consolePatcher.patch(); - registerCleanup(consolePatcher.cleanup); dns.setDefaultResultOrder( validateDnsResolutionOrder(settings.merged.advanced?.dnsResolutionOrder), @@ -350,6 +344,15 @@ export async function main() { process.exit(0); } + // Setup unified ConsolePatcher based on interactive mode + const isInteractive = config.isInteractive(); + const consolePatcher = new ConsolePatcher({ + stderr: isInteractive, + debugMode: isDebugMode, + }); + consolePatcher.patch(); + registerCleanup(consolePatcher.cleanup); + const wasRaw = process.stdin.isRaw; let kittyProtocolDetectionComplete: Promise | undefined; if (config.isInteractive() && !wasRaw && process.stdin.isTTY) { @@ -443,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 8c4fd173..614208b7 100644 --- a/packages/cli/src/nonInteractive/session.ts +++ b/packages/cli/src/nonInteractive/session.ts @@ -16,7 +16,6 @@ */ import type { Config } from '@qwen-code/qwen-code-core'; -import { ConsolePatcher } from '../ui/utils/ConsolePatcher.js'; import { StreamJsonInputReader } from './io/StreamJsonInputReader.js'; import { StreamJsonOutputAdapter } from './io/StreamJsonOutputAdapter.js'; import { ControlContext } from './control/ControlContext.js'; @@ -39,8 +38,9 @@ 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'; const SESSION_STATE = { INITIALIZING: 'initializing', @@ -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; @@ -96,28 +95,17 @@ class SessionManager { private dispatcher: ControlDispatcher | null = null; private controlService: ControlService | null = null; private controlSystemEnabled: boolean | null = null; - private consolePatcher: ConsolePatcher; private debugMode: boolean; 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(); this.initialPrompt = initialPrompt ?? null; - this.consolePatcher = new ConsolePatcher({ - stderr: true, - debugMode: this.debugMode, - }); - this.inputReader = new StreamJsonInputReader(); this.outputAdapter = new StreamJsonOutputAdapter( config, @@ -232,8 +220,6 @@ class SessionManager { */ async run(): Promise { try { - this.consolePatcher.patch(); - if (this.debugMode) { console.error('[SessionManager] Starting session', this.sessionId); } @@ -264,7 +250,6 @@ class SessionManager { await this.shutdown(); throw error; } finally { - this.consolePatcher.cleanup(); // Ensure signal handlers are always cleaned up even if shutdown wasn't called this.cleanupSignalHandlers(); } @@ -578,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) { @@ -695,31 +686,36 @@ 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 { - // Create initial user message from prompt input if provided - let initialPrompt: CLIUserMessage | undefined = undefined; - if (input && input.trim().length > 0) { - const sessionId = config.getSessionId(); - initialPrompt = { - type: 'user', - session_id: sessionId, - message: { - role: 'user', - content: input.trim(), - }, - parent_tool_use_id: null, - }; - } + const consolePatcher = new ConsolePatcher({ + debugMode: config.getDebugMode(), + }); + consolePatcher.patch(); - const manager = new SessionManager(config, settings, initialPrompt); - await manager.run(); + try { + // Create initial user message from prompt input if provided + let initialPrompt: CLIUserMessage | undefined = undefined; + if (input && input.trim().length > 0) { + const sessionId = config.getSessionId(); + initialPrompt = { + type: 'user', + session_id: sessionId, + message: { + role: 'user', + content: input.trim(), + }, + parent_tool_use_id: null, + }; + } + + const manager = new SessionManager(config, initialPrompt); + await manager.run(); + } finally { + consolePatcher.cleanup(); + } } diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 2ff8ea03..8e5a9c90 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -25,7 +25,6 @@ import { StreamJsonOutputAdapter } from './nonInteractive/io/StreamJsonOutputAda import type { ControlService } from './nonInteractive/control/ControlService.js'; import { handleSlashCommand } from './nonInteractiveCliCommands.js'; -import { ConsolePatcher } from './ui/utils/ConsolePatcher.js'; import { handleAtCommand } from './ui/hooks/atCommandProcessor.js'; import { handleError, @@ -67,11 +66,6 @@ export async function runNonInteractive( options: RunNonInteractiveOptions = {}, ): Promise { return promptIdContext.run(prompt_id, async () => { - const consolePatcher = new ConsolePatcher({ - stderr: true, - debugMode: config.getDebugMode(), - }); - // Create output adapter based on format let adapter: JsonOutputAdapterInterface | undefined; const outputFormat = config.getOutputFormat(); @@ -102,12 +96,22 @@ export async function runNonInteractive( } }; + const geminiClient = config.getGeminiClient(); + const abortController = options.abortController ?? new AbortController(); + + // Setup signal handlers for graceful shutdown + const shutdownHandler = () => { + if (config.getDebugMode()) { + console.error('[runNonInteractive] Shutdown signal received'); + } + abortController.abort(); + }; + try { - consolePatcher.patch(); process.stdout.on('error', stdoutErrorHandler); - const geminiClient = config.getGeminiClient(); - const abortController = options.abortController ?? new AbortController(); + process.on('SIGINT', shutdownHandler); + process.on('SIGTERM', shutdownHandler); let initialPartList: PartListUnion | null = extractPartsFromUserMessage( options.userMessage, @@ -362,7 +366,9 @@ export async function runNonInteractive( handleError(error, config); } finally { process.stdout.removeListener('error', stdoutErrorHandler); - consolePatcher.cleanup(); + // Cleanup signal handlers + process.removeListener('SIGINT', shutdownHandler); + process.removeListener('SIGTERM', shutdownHandler); if (isTelemetrySdkInitialized()) { await shutdownTelemetry(config); } diff --git a/packages/cli/src/validateNonInterActiveAuth.test.ts b/packages/cli/src/validateNonInterActiveAuth.test.ts index dba93e62..867777b3 100644 --- a/packages/cli/src/validateNonInterActiveAuth.test.ts +++ b/packages/cli/src/validateNonInterActiveAuth.test.ts @@ -10,6 +10,9 @@ import { AuthType, OutputFormat } from '@qwen-code/qwen-code-core'; import type { Config } from '@qwen-code/qwen-code-core'; import * as auth from './config/auth.js'; import { type LoadedSettings } from './config/settings.js'; +import * as JsonOutputAdapterModule from './nonInteractive/io/JsonOutputAdapter.js'; +import * as StreamJsonOutputAdapterModule from './nonInteractive/io/StreamJsonOutputAdapter.js'; +import * as cleanupModule from './utils/cleanup.js'; describe('validateNonInterActiveAuth', () => { let originalEnvGeminiApiKey: string | undefined; @@ -17,8 +20,8 @@ describe('validateNonInterActiveAuth', () => { let originalEnvGcp: string | undefined; let originalEnvOpenAiApiKey: string | undefined; let consoleErrorSpy: ReturnType; - let processExitSpy: ReturnType; - let refreshAuthMock: vi.Mock; + let processExitSpy: ReturnType>; + let refreshAuthMock: ReturnType; let mockSettings: LoadedSettings; beforeEach(() => { @@ -33,7 +36,7 @@ describe('validateNonInterActiveAuth', () => { consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); processExitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => { throw new Error(`process.exit(${code}) called`); - }); + }) as ReturnType>; refreshAuthMock = vi.fn().mockResolvedValue('refreshed'); mockSettings = { system: { path: '', settings: {} }, @@ -235,7 +238,24 @@ describe('validateNonInterActiveAuth', () => { }); describe('JSON output mode', () => { - it('prints JSON error when no auth is configured and exits with code 1', async () => { + let emitResultMock: ReturnType; + let runExitCleanupMock: ReturnType; + + beforeEach(() => { + emitResultMock = vi.fn(); + runExitCleanupMock = vi.fn().mockResolvedValue(undefined); + vi.spyOn(JsonOutputAdapterModule, 'JsonOutputAdapter').mockImplementation( + () => + ({ + emitResult: emitResultMock, + }) as unknown as JsonOutputAdapterModule.JsonOutputAdapter, + ); + vi.spyOn(cleanupModule, 'runExitCleanup').mockImplementation( + runExitCleanupMock, + ); + }); + + it('emits error result and exits when no auth is configured', async () => { const nonInteractiveConfig = { refreshAuth: refreshAuthMock, getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON), @@ -244,7 +264,6 @@ describe('validateNonInterActiveAuth', () => { .mockReturnValue({ authType: undefined }), } as unknown as Config; - let thrown: Error | undefined; try { await validateNonInteractiveAuth( undefined, @@ -252,21 +271,27 @@ describe('validateNonInterActiveAuth', () => { nonInteractiveConfig, mockSettings, ); + expect.fail('Should have exited'); } catch (e) { - thrown = e as Error; + expect((e as Error).message).toContain('process.exit(1) called'); } - expect(thrown?.message).toBe('process.exit(1) called'); - const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string; - const payload = JSON.parse(errorArg); - expect(payload.error.type).toBe('Error'); - expect(payload.error.code).toBe(1); - expect(payload.error.message).toContain( - 'Please set an Auth method in your', - ); + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: expect.stringContaining( + 'Please set an Auth method in your', + ), + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); - it('prints JSON error when enforced auth mismatches current auth and exits with code 1', async () => { + it('emits error result and exits when enforced auth mismatches current auth', async () => { mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH; process.env['OPENAI_API_KEY'] = 'fake-key'; @@ -278,7 +303,6 @@ describe('validateNonInterActiveAuth', () => { .mockReturnValue({ authType: undefined }), } as unknown as Config; - let thrown: Error | undefined; try { await validateNonInteractiveAuth( undefined, @@ -286,23 +310,27 @@ describe('validateNonInterActiveAuth', () => { nonInteractiveConfig, mockSettings, ); + expect.fail('Should have exited'); } catch (e) { - thrown = e as Error; + expect((e as Error).message).toContain('process.exit(1) called'); } - expect(thrown?.message).toBe('process.exit(1) called'); - { - const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string; - const payload = JSON.parse(errorArg); - expect(payload.error.type).toBe('Error'); - expect(payload.error.code).toBe(1); - expect(payload.error.message).toContain( + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: expect.stringContaining( 'The configured auth type is qwen-oauth, but the current auth type is openai.', - ); - } + ), + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); - it('prints JSON error when validateAuthMethod fails and exits with code 1', async () => { + it('emits error result and exits when validateAuthMethod fails', async () => { vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!'); process.env['OPENAI_API_KEY'] = 'fake-key'; @@ -314,7 +342,6 @@ describe('validateNonInterActiveAuth', () => { .mockReturnValue({ authType: undefined }), } as unknown as Config; - let thrown: Error | undefined; try { await validateNonInteractiveAuth( AuthType.USE_OPENAI, @@ -322,18 +349,159 @@ describe('validateNonInterActiveAuth', () => { nonInteractiveConfig, mockSettings, ); + expect.fail('Should have exited'); } catch (e) { - thrown = e as Error; + expect((e as Error).message).toContain('process.exit(1) called'); } - expect(thrown?.message).toBe('process.exit(1) called'); - { - const errorArg = consoleErrorSpy.mock.calls[0]?.[0] as string; - const payload = JSON.parse(errorArg); - expect(payload.error.type).toBe('Error'); - expect(payload.error.code).toBe(1); - expect(payload.error.message).toBe('Auth error!'); + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: 'Auth error!', + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + }); + + describe('STREAM_JSON output mode', () => { + let emitResultMock: ReturnType; + let runExitCleanupMock: ReturnType; + + beforeEach(() => { + emitResultMock = vi.fn(); + runExitCleanupMock = vi.fn().mockResolvedValue(undefined); + vi.spyOn( + StreamJsonOutputAdapterModule, + 'StreamJsonOutputAdapter', + ).mockImplementation( + () => + ({ + emitResult: emitResultMock, + }) as unknown as StreamJsonOutputAdapterModule.StreamJsonOutputAdapter, + ); + vi.spyOn(cleanupModule, 'runExitCleanup').mockImplementation( + runExitCleanupMock, + ); + }); + + it('emits error result and exits when no auth is configured', async () => { + const nonInteractiveConfig = { + refreshAuth: refreshAuthMock, + getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), + getIncludePartialMessages: vi.fn().mockReturnValue(false), + getContentGeneratorConfig: vi + .fn() + .mockReturnValue({ authType: undefined }), + } as unknown as Config; + + try { + await validateNonInteractiveAuth( + undefined, + undefined, + nonInteractiveConfig, + mockSettings, + ); + expect.fail('Should have exited'); + } catch (e) { + expect((e as Error).message).toContain('process.exit(1) called'); } + + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: expect.stringContaining( + 'Please set an Auth method in your', + ), + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('emits error result and exits when enforced auth mismatches current auth', async () => { + mockSettings.merged.security!.auth!.enforcedType = AuthType.QWEN_OAUTH; + process.env['OPENAI_API_KEY'] = 'fake-key'; + + const nonInteractiveConfig = { + refreshAuth: refreshAuthMock, + getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), + getIncludePartialMessages: vi.fn().mockReturnValue(false), + getContentGeneratorConfig: vi + .fn() + .mockReturnValue({ authType: undefined }), + } as unknown as Config; + + try { + await validateNonInteractiveAuth( + undefined, + undefined, + nonInteractiveConfig, + mockSettings, + ); + expect.fail('Should have exited'); + } catch (e) { + expect((e as Error).message).toContain('process.exit(1) called'); + } + + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: expect.stringContaining( + 'The configured auth type is qwen-oauth, but the current auth type is openai.', + ), + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it('emits error result and exits when validateAuthMethod fails', async () => { + vi.spyOn(auth, 'validateAuthMethod').mockReturnValue('Auth error!'); + process.env['OPENAI_API_KEY'] = 'fake-key'; + + const nonInteractiveConfig = { + refreshAuth: refreshAuthMock, + getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON), + getIncludePartialMessages: vi.fn().mockReturnValue(false), + getContentGeneratorConfig: vi + .fn() + .mockReturnValue({ authType: undefined }), + } as unknown as Config; + + try { + await validateNonInteractiveAuth( + AuthType.USE_OPENAI, + undefined, + nonInteractiveConfig, + mockSettings, + ); + expect.fail('Should have exited'); + } catch (e) { + expect((e as Error).message).toContain('process.exit(1) called'); + } + + expect(emitResultMock).toHaveBeenCalledWith({ + isError: true, + errorMessage: 'Auth error!', + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + expect(runExitCleanupMock).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(1); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/cli/src/validateNonInterActiveAuth.ts b/packages/cli/src/validateNonInterActiveAuth.ts index e44cd0a4..78ccc993 100644 --- a/packages/cli/src/validateNonInterActiveAuth.ts +++ b/packages/cli/src/validateNonInterActiveAuth.ts @@ -9,7 +9,9 @@ import { AuthType, OutputFormat } from '@qwen-code/qwen-code-core'; import { USER_SETTINGS_PATH } from './config/settings.js'; import { validateAuthMethod } from './config/auth.js'; import { type LoadedSettings } from './config/settings.js'; -import { handleError } from './utils/errors.js'; +import { JsonOutputAdapter } from './nonInteractive/io/JsonOutputAdapter.js'; +import { StreamJsonOutputAdapter } from './nonInteractive/io/StreamJsonOutputAdapter.js'; +import { runExitCleanup } from './utils/cleanup.js'; function getAuthTypeFromEnv(): AuthType | undefined { if (process.env['OPENAI_API_KEY']) { @@ -27,7 +29,7 @@ export async function validateNonInteractiveAuth( useExternalAuth: boolean | undefined, nonInteractiveConfig: Config, settings: LoadedSettings, -) { +): Promise { try { const enforcedType = settings.merged.security?.auth?.enforcedType; if (enforcedType) { @@ -58,15 +60,38 @@ export async function validateNonInteractiveAuth( await nonInteractiveConfig.refreshAuth(authType); return nonInteractiveConfig; } catch (error) { - if (nonInteractiveConfig.getOutputFormat() === OutputFormat.JSON) { - handleError( - error instanceof Error ? error : new Error(String(error)), - nonInteractiveConfig, - 1, - ); - } else { - console.error(error instanceof Error ? error.message : String(error)); + const outputFormat = nonInteractiveConfig.getOutputFormat(); + + // In JSON and STREAM_JSON modes, emit error result and exit + if ( + outputFormat === OutputFormat.JSON || + outputFormat === OutputFormat.STREAM_JSON + ) { + let adapter; + if (outputFormat === OutputFormat.JSON) { + adapter = new JsonOutputAdapter(nonInteractiveConfig); + } else { + adapter = new StreamJsonOutputAdapter( + nonInteractiveConfig, + nonInteractiveConfig.getIncludePartialMessages(), + ); + } + const errorMessage = + error instanceof Error ? error.message : String(error); + adapter.emitResult({ + isError: true, + errorMessage, + durationMs: 0, + apiDurationMs: 0, + numTurns: 0, + usage: undefined, + }); + await runExitCleanup(); process.exit(1); } + + // For other modes (text), use existing error handling + console.error(error instanceof Error ? error.message : String(error)); + process.exit(1); } } diff --git a/packages/core/src/subagents/subagent.ts b/packages/core/src/subagents/subagent.ts index e68f77a5..7d161b10 100644 --- a/packages/core/src/subagents/subagent.ts +++ b/packages/core/src/subagents/subagent.ts @@ -620,6 +620,17 @@ export class SubAgentScope { success, error: errorMessage, responseParts: call.response.responseParts, + /** + * Tools like todoWrite will add some extra contents to the result, + * making it unable to deserialize the `responseParts` to a JSON object. + * While `resultDisplay` is normally a string, if not we stringify it, + * so that we can deserialize it to a JSON object when needed. + */ + resultDisplay: call.response.resultDisplay + ? typeof call.response.resultDisplay === 'string' + ? call.response.resultDisplay + : JSON.stringify(call.response.resultDisplay) + : undefined, durationMs: duration, timestamp: Date.now(), } as SubAgentToolResultEvent);