Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 159 additions & 5 deletions packages/cli/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ import { isWorkspaceTrusted } from './trustedFolders.js';

const mockWriteStderrLine = vi.hoisted(() => vi.fn());
const mockWriteStdoutLine = vi.hoisted(() => vi.fn());
const mockSessionServiceInstance = vi.hoisted(() => ({
loadLastSession: vi.fn(),
loadSession: vi.fn(),
forkSession: vi.fn(),
sessionExists: vi.fn(),
}));
const mockSessionServiceCtor = vi.hoisted(() =>
vi.fn(() => mockSessionServiceInstance),
);

vi.mock('../utils/stdioHelpers.js', () => ({
writeStderrLine: mockWriteStderrLine,
Expand Down Expand Up @@ -139,6 +148,7 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
NativeLspService: vi
.fn()
.mockImplementation(() => createNativeLspServiceInstance()),
SessionService: mockSessionServiceCtor,
SkillManager: SkillManagerMock,
IdeClient: {
getInstance: vi.fn().mockResolvedValue({
Expand Down Expand Up @@ -412,6 +422,46 @@ describe('parseArguments', () => {
expect(argv.continue).toBe(true);
});

it('should parse --fork-session with --resume', async () => {
process.argv = [
'node',
'script.js',
'--resume',
'123e4567-e89b-12d3-a456-426614174000',
'--fork-session',
];
const argv = await parseArguments();
expect(argv.resume).toBe('123e4567-e89b-12d3-a456-426614174000');
expect(argv.forkSession).toBe(true);
});

it('should parse --fork-session with the --resume picker form', async () => {
process.argv = ['node', 'script.js', '--resume', '--fork-session'];
const argv = await parseArguments();
// Empty string is the existing yargs shape for picker form: --resume
// without an explicit session ID.
expect(argv.resume).toBe('');
expect(argv.forkSession).toBe(true);
});

it('should reject --fork-session without --resume or --continue', async () => {
process.argv = ['node', 'script.js', '--fork-session'];
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});
mockWriteStderrLine.mockClear();

await expect(parseArguments()).rejects.toThrow('process.exit called');

expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining(
'--fork-session must be used with --resume or --continue',
),
);

mockExit.mockRestore();
});

it('should convert positional query argument to prompt by default', async () => {
process.argv = ['node', 'script.js', 'Hi Gemini'];
const argv = await parseArguments();
Expand Down Expand Up @@ -787,6 +837,14 @@ describe('loadCliConfig', () => {
nativeLspServiceMock.mockImplementation(
() => createNativeLspServiceInstance() as unknown as NativeLspService,
);
mockSessionServiceCtor.mockImplementation(() => mockSessionServiceInstance);
mockSessionServiceInstance.loadLastSession.mockResolvedValue(undefined);
mockSessionServiceInstance.loadSession.mockResolvedValue(undefined);
mockSessionServiceInstance.forkSession.mockResolvedValue({
filePath: '/mock/fork.jsonl',
copiedCount: 1,
});
mockSessionServiceInstance.sessionExists.mockResolvedValue(false);
vi.mocked(os.homedir).mockReturnValue('/mock/home/user');
vi.stubEnv('GEMINI_API_KEY', 'test-api-key');
});
Expand Down Expand Up @@ -853,19 +911,115 @@ describe('loadCliConfig', () => {
expect(config.getIncludePartialMessages()).toBe(true);
});

it('should fork and load a new session when --resume is combined with --fork-session', async () => {
const sourceSessionId = '123e4567-e89b-42d3-a456-426614174000';
const sourceData = {
conversation: { sessionId: sourceSessionId, messages: [] },
uiHistory: [],
};
const forkedData = {
conversation: { sessionId: 'forked-session-id', messages: [] },
uiHistory: [],
};
mockSessionServiceInstance.loadSession.mockImplementation(
async (sessionId: string) => {
if (sessionId === sourceSessionId) return sourceData;
return forkedData;
},
);

const config = await loadCliConfig({}, {
resume: sourceSessionId,
forkSession: true,
} as CliArgs);

expect(mockSessionServiceInstance.forkSession).toHaveBeenCalledWith(
sourceSessionId,
config.getSessionId(),
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] 该断言读取 mock 内部数组 mock.calls[0]?.[1] 来验证 config.getSessionId(),与上方 L936 已断言 forkSession(sourceSessionId, config.getSessionId()) 调用构成同义反复。建议改为更直接、有意义的断言:

Suggested change
);
expect(config.getSessionId()).not.toBe(sourceSessionId);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

expect(config.getSessionId()).toBe(
mockSessionServiceInstance.forkSession.mock.calls[0]?.[1],
);
expect(mockSessionServiceInstance.loadSession).toHaveBeenCalledWith(
config.getSessionId(),
);
});

it('should explain when --fork-session fails to copy the source session', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] 此测试(以及下方 L976、L1007 两个测试)创建了 vi.spyOn(process, 'exit') 但未调用 mockRestore(),与文件中另外 16 处使用 mockRestore() 的模式不一致。虽然 afterEachvi.restoreAllMocks() 会兜底,建议保持一致:在每个测试的 expect(mockExit).toHaveBeenCalledWith(1) 之后添加 mockExit.mockRestore();

— DeepSeek/deepseek-v4-pro via Qwen Code /review

const sourceSessionId = '123e4567-e89b-42d3-a456-426614174000';
const sourceData = {
conversation: { sessionId: sourceSessionId, messages: [] },
uiHistory: [],
};
mockSessionServiceInstance.loadSession.mockResolvedValue(sourceData);
mockSessionServiceInstance.forkSession.mockRejectedValue(
new Error('source session belongs to another project'),
);
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});

await expect(
loadCliConfig({}, {
resume: sourceSessionId,
forkSession: true,
} as CliArgs),
).rejects.toThrow('process.exit called');

expect(mockWriteStderrLine).toHaveBeenCalledWith(
`Failed to fork session ${sourceSessionId}: source session belongs to another project`,
);
expect(mockExit).toHaveBeenCalledWith(1);
});

it('should explain when --continue --fork-session has no saved session to fork', async () => {
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});

await expect(
loadCliConfig({}, {
continue: true,
forkSession: true,
} as CliArgs),
).rejects.toThrow('process.exit called');

expect(mockWriteStderrLine).toHaveBeenCalledWith(
'Cannot use --fork-session with --continue: no saved session found to fork.',
);
expect(mockExit).toHaveBeenCalledWith(1);
});

it('should use internal sandbox session ID without treating it as a new session', async () => {
const sessionId = '123e4567-e89b-12d3-a456-426614174000';
vi.stubEnv('SANDBOX', 'sandbox-exec');
process.argv = ['node', 'script.js', '--sandbox-session-id', sessionId];
const sessionExistsSpy = vi.spyOn(
ServerConfig.SessionService.prototype,
'sessionExists',
);
const argv = await parseArguments();
const settings: Settings = {};
const config = await loadCliConfig(settings, argv);

expect(config.getSessionId()).toBe(sessionId);
expect(sessionExistsSpy).not.toHaveBeenCalled();
expect(mockSessionServiceInstance.sessionExists).not.toHaveBeenCalled();
});

it('should reject direct use of the internal sandbox session ID flag', async () => {
const sessionId = '123e4567-e89b-12d3-a456-426614174000';
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});

process.argv = ['node', 'script.js', '--sandbox-session-id', sessionId];
const argv = await parseArguments();

await expect(loadCliConfig({}, argv)).rejects.toThrow(
'process.exit called',
);

expect(mockWriteStderrLine).toHaveBeenCalledWith(
'--sandbox-session-id is for internal sandbox use only.',
);
expect(mockExit).toHaveBeenCalledWith(1);
expect(mockSessionServiceInstance.sessionExists).not.toHaveBeenCalled();
});

it('should reset context filenames to defaults when context.fileName is not configured', async () => {
Expand Down
46 changes: 45 additions & 1 deletion packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { hideBin } from 'yargs/helpers';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { homedir } from 'node:os';
import { randomUUID } from 'node:crypto';
import stripJsonComments from 'strip-json-comments';

import { resolvePath } from '../utils/resolvePath.js';
Expand Down Expand Up @@ -159,6 +160,11 @@ export interface CliArgs {
resume: string | undefined;
/** Specify a session ID without session resumption */
sessionId: string | undefined;
/**
* Create a new forked session from the resumed session. Must be used with
* --resume or --continue.
*/
forkSession?: boolean | undefined;
/** Internal: preserve the outer session ID when relaunching in a sandbox */
sandboxSessionId?: string | undefined;
maxSessionTurns: number | undefined;
Expand Down Expand Up @@ -805,6 +811,12 @@ export async function parseArguments(): Promise<CliArgs> {
type: 'string',
description: 'Specify a session ID for this run.',
})
.option('fork-session', {
type: 'boolean',
description:
'Create a new forked session from the resumed session. Must be used with --resume or --continue.',
default: false,
})
.option('sandbox-session-id', {
type: 'string',
hidden: true,
Expand Down Expand Up @@ -906,9 +918,13 @@ export async function parseArguments(): Promise<CliArgs> {
if (argv['continue'] && argv['resume']) {
return 'Cannot use both --continue and --resume together. Use --continue to resume the latest session, or --resume <sessionId> to resume a specific session.';
}
if (argv['sessionId'] && (argv['continue'] || argv['resume'])) {
const hasResume = argv['resume'] !== undefined;
if (argv['sessionId'] && (argv['continue'] || hasResume)) {
return 'Cannot use --session-id with --continue or --resume. Use --session-id to start a new session with a specific ID, or use --continue/--resume to resume an existing session.';
}
if (argv['forkSession'] && !(argv['continue'] || hasResume)) {
return '--fork-session must be used with --resume or --continue.';
}
if (
argv['sandboxSessionId'] &&
(argv['sessionId'] || argv['continue'] || argv['resume'])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] hasResume 变量在第 921 行引入用于正确识别 picker 模式(--resume 无参产生 ''),但此处 sandbox-session-id 互斥检查仍直接使用 argv['resume'](falsy 值),与上方 L922/925 的 hasResume 用法不一致。

Suggested change
(argv['sessionId'] || argv['continue'] || argv['resume'])
if (
argv['sandboxSessionId'] &&
(argv['sessionId'] || argv['continue'] || hasResume)
) {

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Expand Down Expand Up @@ -1535,6 +1551,11 @@ export async function loadCliConfig(
sessionData = await sessionService.loadLastSession();
if (sessionData) {
sessionId = sessionData.conversation.sessionId;
} else if (argv.forkSession) {
writeStderrLine(
'Cannot use --fork-session with --continue: no saved session found to fork.',
);
process.exit(1);
}
}

Expand All @@ -1550,7 +1571,30 @@ export async function loadCliConfig(
process.exit(1);
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] sessionData 在 L1568 经 loadSession 加载源会话数据,在 fork block 中又被 loadSession(forkedSessionId) 覆盖。无注释说明源 sessionData 在 fork 路径下会被丢弃。未来若有人在 resume 加载和 fork 之间插入对 sessionData 的修改,该修改将在 fork 路径下静默丢失。建议添加注释或使用独立变量名(如 forkedSessionData)。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

if (argv.forkSession && sessionId) {
const sourceSessionId = sessionId;
const forkedSessionId = randomUUID();
try {
await sessionService.forkSession(sourceSessionId, forkedSessionId);
} catch (err) {
writeStderrLine(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] forkSession 失败时 err.message 可能包含原生 Node.js 错误中的绝对文件系统路径(如 ENOENT: no such file or directory, open '/Users/.../session.jsonl'),直接写入 stderr 会泄露内部目录结构。建议对错误消息做路径脱敏,或仅输出不含路径的通用失败提示。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

`Failed to fork session ${sourceSessionId}: ${err instanceof Error ? err.message : String(err)}`,
);
process.exit(1);
}
sessionId = forkedSessionId;
sessionData = await sessionService.loadSession(forkedSessionId);
if (!sessionData) {
writeStderrLine(`Failed to load forked session ${forkedSessionId}.`);
process.exit(1);
}
}
} else if (argv.sandboxSessionId) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] --sandbox-session-id is accepted from any CLI invocation — hidden: true only hides it from --help. The branch sets sessionId = argv.sandboxSessionId without the sessionService.sessionExists() collision check that --session-id performs just below, and without requiring process.env['SANDBOX'] (the codebase already gates sandbox-child behavior on this env var at gemini.tsx:462).

Consider adding a guard: reject the flag when process.env['SANDBOX'] is not set, or at minimum document why the collision guard is intentionally skipped here (the parent process already owns this UUID, but that invariant is not enforced in code).

Suggested change
} else if (argv.sandboxSessionId) {
} else if (argv.sandboxSessionId) {
// Sandbox relaunch handoff: the parent process already owns this UUID.
// Reject direct invocations (not inside the sandbox) to match the
// documented "internal" semantics.
if (!process.env['SANDBOX']) {
writeStderrLine(
'--sandbox-session-id is for internal sandbox use only.',
);
process.exit(1);
}
sessionId = argv.sandboxSessionId;

— claude-opus-4-7 via Qwen Code /review

if (!process.env['SANDBOX']) {
writeStderrLine('--sandbox-session-id is for internal sandbox use only.');
process.exit(1);
}
sessionId = argv.sandboxSessionId;
} else if (argv['sessionId']) {
// Use provided session ID without session resumption
Expand Down
Loading