refactor: update test structure and clean up unused code in cli and sdk

This commit is contained in:
mingholy.lmh
2025-11-25 11:45:34 +08:00
parent ad9ba914e1
commit ac6aecb622
14 changed files with 2620 additions and 401 deletions

View File

@@ -442,7 +442,7 @@ export class PermissionController extends BaseController {
// On error, use default cancel message // On error, use default cancel message
// Only pass payload for exec and mcp types that support it // Only pass payload for exec and mcp types that support it
const confirmationType = toolCall.confirmationDetails.type; const confirmationType = toolCall.confirmationDetails.type;
if (confirmationType === 'exec' || confirmationType === 'mcp') { if (['edit', 'exec', 'mcp'].includes(confirmationType)) {
const execOrMcpDetails = toolCall.confirmationDetails as const execOrMcpDetails = toolCall.confirmationDetails as
| ToolExecuteConfirmationDetails | ToolExecuteConfirmationDetails
| ToolMcpConfirmationDetails; | ToolMcpConfirmationDetails;

View File

@@ -134,7 +134,7 @@ function createControlCancel(requestId: string): ControlCancelRequest {
}; };
} }
describe('runNonInteractiveStreamJson (refactored)', () => { describe('runNonInteractiveStreamJson', () => {
let config: Config; let config: Config;
let mockInputReader: { let mockInputReader: {
read: () => AsyncGenerator< read: () => AsyncGenerator<

View File

@@ -299,12 +299,6 @@ export interface CLIControlPermissionRequest {
blocked_path: string | null; blocked_path: string | null;
} }
export enum AuthProviderType {
DYNAMIC_DISCOVERY = 'dynamic_discovery',
GOOGLE_CREDENTIALS = 'google_credentials',
SERVICE_ACCOUNT_IMPERSONATION = 'service_account_impersonation',
}
export interface CLIControlInitializeRequest { export interface CLIControlInitializeRequest {
subtype: 'initialize'; subtype: 'initialize';
hooks?: HookRegistration[] | null; hooks?: HookRegistration[] | null;

View File

@@ -855,13 +855,6 @@ export class Config {
return this.mcpServers; return this.mcpServers;
} }
setMcpServers(servers: Record<string, MCPServerConfig>): void {
if (this.initialized) {
throw new Error('Cannot modify mcpServers after initialization');
}
this.mcpServers = servers;
}
addMcpServers(servers: Record<string, MCPServerConfig>): void { addMcpServers(servers: Record<string, MCPServerConfig>): void {
if (this.initialized) { if (this.initialized) {
throw new Error('Cannot modify mcpServers after initialization'); throw new Error('Cannot modify mcpServers after initialization');

View File

@@ -1,5 +1,5 @@
export { query } from './query/createQuery.js'; export { query } from './query/createQuery.js';
export { AbortError, isAbortError } from './types/errors.js';
export { Query } from './query/Query.js'; export { Query } from './query/Query.js';
export type { ExternalMcpServerConfig } from './types/queryOptionsSchema.js'; export type { ExternalMcpServerConfig } from './types/queryOptionsSchema.js';

View File

@@ -21,28 +21,20 @@ export function query({
prompt: string | AsyncIterable<CLIUserMessage>; prompt: string | AsyncIterable<CLIUserMessage>;
options?: QueryOptions; options?: QueryOptions;
}): Query { }): Query {
// Validate options and obtain normalized executable metadata
const parsedExecutable = validateOptions(options); const parsedExecutable = validateOptions(options);
// Determine if this is a single-turn or multi-turn query
// Single-turn: string prompt (simple Q&A)
// Multi-turn: AsyncIterable prompt (streaming conversation)
const isSingleTurn = typeof prompt === 'string'; const isSingleTurn = typeof prompt === 'string';
// Resolve CLI specification while preserving explicit runtime directives
const pathToQwenExecutable = const pathToQwenExecutable =
options.pathToQwenExecutable ?? parsedExecutable.executablePath; options.pathToQwenExecutable ?? parsedExecutable.executablePath;
// Use provided abortController or create a new one
const abortController = options.abortController ?? new AbortController(); const abortController = options.abortController ?? new AbortController();
// Create transport with abortController
const transport = new ProcessTransport({ const transport = new ProcessTransport({
pathToQwenExecutable, pathToQwenExecutable,
cwd: options.cwd, cwd: options.cwd,
model: options.model, model: options.model,
permissionMode: options.permissionMode, permissionMode: options.permissionMode,
mcpServers: options.mcpServers,
env: options.env, env: options.env,
abortController, abortController,
debug: options.debug, debug: options.debug,
@@ -53,18 +45,14 @@ export function query({
authType: options.authType, authType: options.authType,
}); });
// Build query options with abortController
const queryOptions: QueryOptions = { const queryOptions: QueryOptions = {
...options, ...options,
abortController, abortController,
}; };
// Create Query
const queryInstance = new Query(transport, queryOptions, isSingleTurn); const queryInstance = new Query(transport, queryOptions, isSingleTurn);
// Handle prompt based on type
if (isSingleTurn) { if (isSingleTurn) {
// For single-turn queries, send the prompt directly via transport
const stringPrompt = prompt as string; const stringPrompt = prompt as string;
const message: CLIUserMessage = { const message: CLIUserMessage = {
type: 'user', type: 'user',
@@ -95,16 +83,9 @@ export function query({
return queryInstance; return queryInstance;
} }
/**
* Backward compatibility alias
* @deprecated Use query() instead
*/
export const createQuery = query;
function validateOptions( function validateOptions(
options: QueryOptions, options: QueryOptions,
): ReturnType<typeof parseExecutableSpec> { ): ReturnType<typeof parseExecutableSpec> {
// Validate options using Zod schema
const validationResult = QueryOptionsSchema.safeParse(options); const validationResult = QueryOptionsSchema.safeParse(options);
if (!validationResult.success) { if (!validationResult.success) {
const errors = validationResult.error.errors const errors = validationResult.error.errors
@@ -113,7 +94,6 @@ function validateOptions(
throw new Error(`Invalid QueryOptions: ${errors}`); throw new Error(`Invalid QueryOptions: ${errors}`);
} }
// Validate executable path early to provide clear error messages
let parsedExecutable: ReturnType<typeof parseExecutableSpec>; let parsedExecutable: ReturnType<typeof parseExecutableSpec>;
try { try {
parsedExecutable = parseExecutableSpec(options.pathToQwenExecutable); parsedExecutable = parseExecutableSpec(options.pathToQwenExecutable);
@@ -122,7 +102,6 @@ function validateOptions(
throw new Error(`Invalid pathToQwenExecutable: ${errorMessage}`); throw new Error(`Invalid pathToQwenExecutable: ${errorMessage}`);
} }
// Validate no MCP server name conflicts (cross-field validation not easily expressible in Zod)
if (options.mcpServers && options.sdkMcpServers) { if (options.mcpServers && options.sdkMcpServers) {
const externalNames = Object.keys(options.mcpServers); const externalNames = Object.keys(options.mcpServers);
const sdkNames = Object.keys(options.sdkMcpServers); const sdkNames = Object.keys(options.sdkMcpServers);

View File

@@ -7,11 +7,6 @@ import { parseJsonLinesStream } from '../utils/jsonLines.js';
import { prepareSpawnInfo } from '../utils/cliPath.js'; import { prepareSpawnInfo } from '../utils/cliPath.js';
import { AbortError } from '../types/errors.js'; import { AbortError } from '../types/errors.js';
type ExitListener = {
callback: (error?: Error) => void;
handler: (code: number | null, signal: NodeJS.Signals | null) => void;
};
export class ProcessTransport implements Transport { export class ProcessTransport implements Transport {
private childProcess: ChildProcess | null = null; private childProcess: ChildProcess | null = null;
private childStdin: Writable | null = null; private childStdin: Writable | null = null;
@@ -21,7 +16,6 @@ export class ProcessTransport implements Transport {
private _exitError: Error | null = null; private _exitError: Error | null = null;
private closed = false; private closed = false;
private abortController: AbortController; private abortController: AbortController;
private exitListeners: ExitListener[] = [];
private processExitHandler: (() => void) | null = null; private processExitHandler: (() => void) | null = null;
private abortHandler: (() => void) | null = null; private abortHandler: (() => void) | null = null;
@@ -115,15 +109,6 @@ export class ProcessTransport implements Transport {
this.logForDebugging(error.message); this.logForDebugging(error.message);
} }
} }
const error = this._exitError;
for (const listener of this.exitListeners) {
try {
listener.callback(error || undefined);
} catch (err) {
this.logForDebugging(`Exit listener error: ${err}`);
}
}
}); });
} }
@@ -192,11 +177,6 @@ export class ProcessTransport implements Transport {
this.abortHandler = null; this.abortHandler = null;
} }
for (const { handler } of this.exitListeners) {
this.childProcess?.off('close', handler);
}
this.exitListeners = [];
if (this.childProcess && !this.childProcess.killed) { if (this.childProcess && !this.childProcess.killed) {
this.childProcess.kill('SIGTERM'); this.childProcess.kill('SIGTERM');
setTimeout(() => { setTimeout(() => {
@@ -343,30 +323,6 @@ export class ProcessTransport implements Transport {
return this._exitError; return this._exitError;
} }
onExit(callback: (error?: Error) => void): () => void {
if (!this.childProcess) {
return () => {};
}
const handler = (code: number | null, signal: NodeJS.Signals | null) => {
const error = this.getProcessExitError(code, signal);
callback(error);
};
this.childProcess.on('close', handler);
this.exitListeners.push({ callback, handler });
return () => {
if (this.childProcess) {
this.childProcess.off('close', handler);
}
const index = this.exitListeners.findIndex((l) => l.handler === handler);
if (index !== -1) {
this.exitListeners.splice(index, 1);
}
};
}
endInput(): void { endInput(): void {
if (this.childStdin) { if (this.childStdin) {
this.childStdin.end(); this.childStdin.end();

View File

@@ -1,5 +1,4 @@
import type { PermissionMode, PermissionSuggestion } from './protocol.js'; import type { PermissionMode, PermissionSuggestion } from './protocol.js';
import type { ExternalMcpServerConfig } from './queryOptionsSchema.js';
export type { PermissionMode }; export type { PermissionMode };
@@ -23,7 +22,6 @@ export type TransportOptions = {
cwd?: string; cwd?: string;
model?: string; model?: string;
permissionMode?: PermissionMode; permissionMode?: PermissionMode;
mcpServers?: Record<string, ExternalMcpServerConfig>;
env?: Record<string, string>; env?: Record<string, string>;
abortController?: AbortController; abortController?: AbortController;
debug?: boolean; debug?: boolean;

View File

@@ -1,7 +1,3 @@
/**
* Async iterable queue for streaming messages between producer and consumer.
*/
export class Stream<T> implements AsyncIterable<T> { export class Stream<T> implements AsyncIterable<T> {
private returned: (() => void) | undefined; private returned: (() => void) | undefined;
private queue: T[] = []; private queue: T[] = [];
@@ -24,23 +20,18 @@ export class Stream<T> implements AsyncIterable<T> {
} }
async next(): Promise<IteratorResult<T>> { async next(): Promise<IteratorResult<T>> {
// Check queue first - if there are queued items, return immediately
if (this.queue.length > 0) { if (this.queue.length > 0) {
return Promise.resolve({ return Promise.resolve({
done: false, done: false,
value: this.queue.shift()!, value: this.queue.shift()!,
}); });
} }
// Check if stream is done
if (this.isDone) { if (this.isDone) {
return Promise.resolve({ done: true, value: undefined }); return Promise.resolve({ done: true, value: undefined });
} }
// Check for errors that occurred before next() was called
// This ensures errors set via error() before iteration starts are properly rejected
if (this.hasError) { if (this.hasError) {
return Promise.reject(this.hasError); return Promise.reject(this.hasError);
} }
// No queued items, not done, no error - set up promise for next value/error
return new Promise<IteratorResult<T>>((resolve, reject) => { return new Promise<IteratorResult<T>>((resolve, reject) => {
this.readResolve = resolve; this.readResolve = resolve;
this.readReject = reject; this.readReject = reject;
@@ -70,15 +61,12 @@ export class Stream<T> implements AsyncIterable<T> {
error(error: Error): void { error(error: Error): void {
this.hasError = error; this.hasError = error;
// If readReject exists (next() has been called), reject immediately
if (this.readReject) { if (this.readReject) {
const reject = this.readReject; const reject = this.readReject;
this.readResolve = undefined; this.readResolve = undefined;
this.readReject = undefined; this.readReject = undefined;
reject(error); reject(error);
} }
// Otherwise, error is stored in hasError and will be rejected when next() is called
// This handles the case where error() is called before the first next() call
} }
return(): Promise<IteratorResult<T>> { return(): Promise<IteratorResult<T>> {

View File

@@ -154,7 +154,6 @@ export function parseExecutableSpec(executableSpec?: string): {
executablePath: string; executablePath: string;
isExplicitRuntime: boolean; isExplicitRuntime: boolean;
} { } {
// Handle empty string case first (before checking for undefined/null)
if ( if (
executableSpec === '' || executableSpec === '' ||
(executableSpec && executableSpec.trim() === '') (executableSpec && executableSpec.trim() === '')
@@ -163,7 +162,6 @@ export function parseExecutableSpec(executableSpec?: string): {
} }
if (!executableSpec) { if (!executableSpec) {
// Auto-detect native CLI
return { return {
executablePath: findNativeCliPath(), executablePath: findNativeCliPath(),
isExplicitRuntime: false, isExplicitRuntime: false,
@@ -178,7 +176,6 @@ export function parseExecutableSpec(executableSpec?: string): {
throw new Error(`Invalid runtime specification: '${executableSpec}'`); throw new Error(`Invalid runtime specification: '${executableSpec}'`);
} }
// Validate runtime is supported
const supportedRuntimes = ['node', 'bun', 'tsx', 'deno']; const supportedRuntimes = ['node', 'bun', 'tsx', 'deno'];
if (!supportedRuntimes.includes(runtime)) { if (!supportedRuntimes.includes(runtime)) {
throw new Error( throw new Error(
@@ -186,7 +183,6 @@ export function parseExecutableSpec(executableSpec?: string): {
); );
} }
// Validate runtime availability
if (!validateRuntimeAvailability(runtime)) { if (!validateRuntimeAvailability(runtime)) {
throw new Error( throw new Error(
`Runtime '${runtime}' is not available on this system. Please install it first.`, `Runtime '${runtime}' is not available on this system. Please install it first.`,
@@ -195,7 +191,6 @@ export function parseExecutableSpec(executableSpec?: string): {
const resolvedPath = path.resolve(filePath); const resolvedPath = path.resolve(filePath);
// Validate file exists
if (!fs.existsSync(resolvedPath)) { if (!fs.existsSync(resolvedPath)) {
throw new Error( throw new Error(
`Executable file not found at '${resolvedPath}' for runtime '${runtime}'. ` + `Executable file not found at '${resolvedPath}' for runtime '${runtime}'. ` +
@@ -203,7 +198,6 @@ export function parseExecutableSpec(executableSpec?: string): {
); );
} }
// Validate file extension matches runtime
if (!validateFileExtensionForRuntime(resolvedPath, runtime)) { if (!validateFileExtensionForRuntime(resolvedPath, runtime)) {
const ext = path.extname(resolvedPath); const ext = path.extname(resolvedPath);
throw new Error( throw new Error(
@@ -285,14 +279,6 @@ function getExpectedExtensions(runtime: string): string[] {
} }
} }
/**
* @deprecated Use parseExecutableSpec and prepareSpawnInfo instead
*/
export function resolveCliPath(explicitPath?: string): string {
const parsed = parseExecutableSpec(explicitPath);
return parsed.executablePath;
}
function detectRuntimeFromExtension(filePath: string): string | undefined { function detectRuntimeFromExtension(filePath: string): string | undefined {
const ext = path.extname(filePath).toLowerCase(); const ext = path.extname(filePath).toLowerCase();
@@ -356,10 +342,3 @@ export function prepareSpawnInfo(executableSpec?: string): SpawnInfo {
originalInput: executableSpec || '', originalInput: executableSpec || '',
}; };
} }
/**
* @deprecated Use prepareSpawnInfo() instead
*/
export function findCliPath(): string {
return findNativeCliPath();
}

View File

@@ -38,20 +38,16 @@ export async function* parseJsonLinesStream(
context = 'JsonLines', context = 'JsonLines',
): AsyncGenerator<unknown, void, unknown> { ): AsyncGenerator<unknown, void, unknown> {
for await (const line of lines) { for await (const line of lines) {
// Skip empty lines
if (line.trim().length === 0) { if (line.trim().length === 0) {
continue; continue;
} }
// Parse with error handling
const message = parseJsonLineSafe(line, context); const message = parseJsonLineSafe(line, context);
// Skip malformed messages
if (message === null) { if (message === null) {
continue; continue;
} }
// Validate message structure
if (!isValidMessage(message)) { if (!isValidMessage(message)) {
console.warn( console.warn(
`[${context}] Invalid message structure (missing 'type' field), skipping:`, `[${context}] Invalid message structure (missing 'type' field), skipping:`,

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@@ -11,7 +11,6 @@ import {
parseExecutableSpec, parseExecutableSpec,
prepareSpawnInfo, prepareSpawnInfo,
findNativeCliPath, findNativeCliPath,
resolveCliPath,
} from '../../src/utils/cliPath.js'; } from '../../src/utils/cliPath.js';
// Mock fs module // Mock fs module
@@ -421,28 +420,6 @@ describe('CLI Path Utilities', () => {
}); });
}); });
describe('resolveCliPath (backward compatibility)', () => {
it('should resolve CLI path for backward compatibility', () => {
mockFs.existsSync.mockReturnValue(true);
const result = resolveCliPath('/path/to/qwen');
expect(result).toBe('/path/to/qwen');
});
it('should auto-detect when no path provided', () => {
const originalEnv = process.env['QWEN_CODE_CLI_PATH'];
process.env['QWEN_CODE_CLI_PATH'] = '/usr/local/bin/qwen';
mockFs.existsSync.mockReturnValue(true);
const result = resolveCliPath();
expect(result).toBe('/usr/local/bin/qwen');
process.env['QWEN_CODE_CLI_PATH'] = originalEnv;
});
});
describe('real-world use cases', () => { describe('real-world use cases', () => {
beforeEach(() => { beforeEach(() => {
mockFs.existsSync.mockReturnValue(true); mockFs.existsSync.mockReturnValue(true);