diff --git a/.github/assets/exit-resume-echo/01-baseline-v0.21.4-blank-after-exit.png b/.github/assets/exit-resume-echo/01-baseline-v0.21.4-blank-after-exit.png new file mode 100644 index 0000000000..7451ff72b4 Binary files /dev/null and b/.github/assets/exit-resume-echo/01-baseline-v0.21.4-blank-after-exit.png differ diff --git a/.github/assets/exit-resume-echo/02-pr-exit-echo.png b/.github/assets/exit-resume-echo/02-pr-exit-echo.png new file mode 100644 index 0000000000..0644d2523f Binary files /dev/null and b/.github/assets/exit-resume-echo/02-pr-exit-echo.png differ diff --git a/.github/assets/exit-resume-echo/03-pr-resumed-session.png b/.github/assets/exit-resume-echo/03-pr-resumed-session.png new file mode 100644 index 0000000000..245fba8a2a Binary files /dev/null and b/.github/assets/exit-resume-echo/03-pr-resumed-session.png differ diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index cdcfe87655..c995ac20bc 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -15,6 +15,7 @@ import { Storage, } from '@qwen-code/qwen-code-core'; import { + isValidSessionId, loadCliConfig, parseArguments, SessionIdConflictError, @@ -253,6 +254,29 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { }; }); +describe('isValidSessionId', () => { + it.each([ + ['a canonical UUID', 'b2a1c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d'], + [ + 'an agent-suffixed UUID', + 'b2a1c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d-agent-qwen', + ], + ])('accepts %s', (_, value) => { + expect(isValidSessionId(value)).toBe(true); + }); + + // These shapes are paste-into-shell payloads for the exit-time resume + // echo, so the production gate must reject them. + it.each([ + ['newline', 'evil\nrm -rf ~'], + ['escape sequence', 'evil\u001B]52;c;pwned\u0007session'], + ['leading dash', '-cafebabe0123456789abcdef01234567'], + ['non-UUID token', 'abc123'], + ])('rejects a payload with a %s', (_, value) => { + expect(isValidSessionId(value)).toBe(false); + }); +}); + describe('parseArguments', () => { const originalArgv = process.argv; diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 6a64c76f9b..421cfc8337 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -13,7 +13,15 @@ import { afterEach, type MockInstance, } from 'vitest'; -import { readFileSync } from 'node:fs'; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { createNonInteractivePromptId, main, @@ -31,6 +39,7 @@ import { ApprovalMode, OutputFormat } from '@qwen-code/qwen-code-core'; import { EXTERNAL_TOOL_GUARD_REQUIRED_VALUE } from '@qwen-code/acp-bridge/externalToolGuard'; const mockWriteStderrLine = vi.hoisted(() => vi.fn()); +const mockWriteStdoutLine = vi.hoisted(() => vi.fn()); const mockConsumeLastRenderError = vi.hoisted(() => vi.fn()); const mockHandleListExtensions = vi.hoisted(() => vi.fn()); const mockStartEarlyStartupPrefetches = vi.hoisted(() => vi.fn()); @@ -103,6 +112,12 @@ vi.mock('./config/config.js', () => ({ parseArguments: vi.fn().mockResolvedValue({}), isDebugMode: vi.fn(() => false), buildDisabledSkillNamesProvider: vi.fn(() => () => new Set()), + // Mirrors SESSION_ID_REGEX in ./config/config.ts; keep them in sync. + isValidSessionId: vi.fn((value: string) => + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}(-agent-[a-zA-Z0-9_.-]+)?$/i.test( + value, + ), + ), })); vi.mock('read-package-up', () => ({ @@ -135,7 +150,7 @@ vi.mock('./utils/sandbox.js', () => ({ vi.mock('./utils/stdioHelpers.js', () => ({ writeStderrLine: mockWriteStderrLine, - writeStdoutLine: vi.fn(), + writeStdoutLine: mockWriteStdoutLine, clearScreen: vi.fn(), })); @@ -2299,6 +2314,7 @@ describe('startInteractiveUI', () => { getProjectRoot: () => '/root', getScreenReader: () => false, isTelemetryInitializationDeferred: () => true, + getChatRecordingService: () => undefined, } as unknown as Config; const mockSettings = { merged: { @@ -2792,6 +2808,179 @@ describe('startInteractiveUI', () => { ); }); + // The quit screen's resume hint is drawn on the alternate screen in VP + // mode and discarded on teardown, so cleanup echoes the command to the + // main screen. Pin the echo's gate, message shape, and paste-safe ID gate. + describe('exit-time resume echo', () => { + async function runCleanup(config: Config): Promise { + const unmount = vi.fn(); + const { render } = await import('ink'); + vi.mocked(render).mockReturnValue({ unmount } as never); + mockConsumeLastRenderError.mockReturnValue(undefined); + + await startInteractiveUI( + config, + mockSettings, + mockStartupWarnings, + mockWorkspaceRoot, + { + authError: null, + themeError: null, + shouldOpenAuthDialog: false, + geminiMdFileCount: 0, + }, + ); + + const { registerCleanup } = await import('./utils/cleanup.js'); + const cleanupFn = vi.mocked(registerCleanup).mock.calls.at(-1)?.[0] as + | (() => Promise | void) + | undefined; + expect(cleanupFn).toBeTypeOf('function'); + await cleanupFn?.(); + } + + function makeRecordingConfig(sessionId: string, sessionFile: string) { + return { + ...mockConfig, + getChatRecordingService: () => ({}), + getSessionId: () => sessionId, + getTranscriptPath: () => sessionFile, + } as unknown as Config; + } + + it.each([ + ['a canonical session ID', 'b2a1c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d'], + // Arena's agent-suffixed IDs are also accepted by --resume. + [ + 'an agent-suffixed session ID', + 'b2a1c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d-agent-qwen', + ], + ])('echoes the resume command for %s', async (_, sessionId) => { + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + mkdirSync(join(projectDir, 'chats'), { recursive: true }); + const sessionFile = join(projectDir, 'chats', `${sessionId}.jsonl`); + writeFileSync(sessionFile, '{"type":"message"}\n'); + + try { + await runCleanup(makeRecordingConfig(sessionId, sessionFile)); + + // Match only the locale-independent command part: earlier main() + // tests run the real initializeI18n('auto') and leave the machine + // locale's dictionary in the i18n module state. + expect(mockWriteStdoutLine).toHaveBeenCalledWith( + expect.stringContaining(`qwen --resume ${sessionId}`), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + it('does not echo when the transcript file is empty', async () => { + const sessionId = '99999999-8888-4777-a666-555555555555'; + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + mkdirSync(join(projectDir, 'chats'), { recursive: true }); + const sessionFile = join(projectDir, 'chats', `${sessionId}.jsonl`); + writeFileSync(sessionFile, ''); + + try { + await runCleanup(makeRecordingConfig(sessionId, sessionFile)); + + expect(mockWriteStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('qwen --resume'), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + it('does not echo when the session file is missing', async () => { + const sessionId = '11111111-2222-4333-8444-555555555555'; + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + const sessionFile = join(projectDir, 'chats', `${sessionId}.jsonl`); + + try { + await runCleanup(makeRecordingConfig(sessionId, sessionFile)); + + expect(mockWriteStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('qwen --resume'), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + // The ID is echoed as paste-into-shell text, and resume reads session + // IDs from transcript contents, so a crafted transcript must not be + // able to smuggle extra commands or CLI flags past the echo. + it.each([ + ['newline', 'evil\nrm -rf ~'], + ['escape sequence', 'evil\u001B]52;c;pwned\u0007session'], + ['leading dash', '-cafebabe0123456789abcdef01234567'], + ['non-UUID token', 'abc123'], + ])('does not echo a session ID with a %s', async (_, sessionId) => { + // Pair each hostile ID with a real non-empty transcript under a + // benign filename so only the ID gate can suppress the echo; a + // missing file would mask a regression of the gate itself. + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + mkdirSync(join(projectDir, 'chats'), { recursive: true }); + const sessionFile = join(projectDir, 'chats', 'benign.jsonl'); + writeFileSync(sessionFile, '{"type":"message"}\n'); + + try { + await runCleanup(makeRecordingConfig(sessionId, sessionFile)); + + expect(mockWriteStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('qwen --resume'), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + it('does not echo when chat recording is disabled', async () => { + const sessionId = 'aaaaaaaa-bbbb-4ccc-9ddd-eeeeeeeeeeee'; + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + mkdirSync(join(projectDir, 'chats'), { recursive: true }); + const sessionFile = join(projectDir, 'chats', `${sessionId}.jsonl`); + writeFileSync(sessionFile, '{"type":"message"}\n'); + + try { + await runCleanup({ + ...makeRecordingConfig(sessionId, sessionFile), + getChatRecordingService: () => undefined, + } as unknown as Config); + + expect(mockWriteStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('qwen --resume'), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + it('does not echo when stdout is not a TTY', async () => { + Object.defineProperty(process.stdout, 'isTTY', { + value: false, + configurable: true, + }); + const sessionId = '0f0e0d0c-0b0a-4908-8706-050403020100'; + const projectDir = mkdtempSync(join(tmpdir(), 'resume-echo-')); + mkdirSync(join(projectDir, 'chats'), { recursive: true }); + const sessionFile = join(projectDir, 'chats', `${sessionId}.jsonl`); + writeFileSync(sessionFile, '{"type":"message"}\n'); + + try { + await runCleanup(makeRecordingConfig(sessionId, sessionFile)); + + expect(mockWriteStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('qwen --resume'), + ); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + }); + describe('periodic memory-pressure check', () => { beforeEach(() => { vi.useFakeTimers(); diff --git a/packages/cli/src/ui/startInteractiveUI.tsx b/packages/cli/src/ui/startInteractiveUI.tsx index d27b93374b..d8c476fc35 100644 --- a/packages/cli/src/ui/startInteractiveUI.tsx +++ b/packages/cli/src/ui/startInteractiveUI.tsx @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { stat } from 'node:fs/promises'; import { basename } from 'node:path'; import { render } from 'ink'; import React from 'react'; @@ -14,6 +15,7 @@ import { writeRuntimeStatus, } from '@qwen-code/qwen-code-core'; import type { LoadedSettings } from '../config/settings.js'; +import { isValidSessionId } from '../config/config.js'; import type { InitializationResult } from '../core/initializer.js'; import type { ExtensionRefreshState } from '../config/extension-refresh-state.js'; import { DualOutputBridge } from '../dualOutput/DualOutputBridge.js'; @@ -44,8 +46,9 @@ import { } from './components/shared/ErrorBoundary.js'; import { registerCleanup, runExitCleanup } from '../utils/cleanup.js'; import { stopAndGetCapturedInput } from '../utils/earlyInputCapture.js'; +import { t } from '../i18n/index.js'; import { profileCheckpoint } from '../utils/startupProfiler.js'; -import { writeStderrLine } from '../utils/stdioHelpers.js'; +import { writeStderrLine, writeStdoutLine } from '../utils/stdioHelpers.js'; import { sanitizeTerminalText } from './utils/textUtils.js'; import { startPostRenderPrefetches } from '../startup/startup-prefetch.js'; import { @@ -324,6 +327,39 @@ export async function startInteractiveUI( `\nRendering error${loggedHint}: ${sanitizeTerminalText(renderError.message)}`, ); } + // Same reasoning as the render-error echo above: the quit screen (with + // its resume hint) is drawn on the alternate screen in VP mode and is + // discarded on teardown, so echo the resume command here where it + // survives exit and can be copied from the terminal scrollback. + // --resume lookup is cwd-scoped, so the hint assumes it is pasted in + // the session's working directory. Sessions keyed elsewhere share this + // limitation with the in-TUI hint — notably --worktree startup, which + // chdirs into the worktree while the user's shell stays at the launch + // directory. + try { + if (process.stdout.isTTY && config.getChatRecordingService()) { + const sessionId = config.getSessionId(); + const sessionFile = config.getTranscriptPath(); + // The echoed ID is paste-into-shell text, and resume reads session + // IDs from transcript contents any user-level process can write. + // Gate to the canonical shape `--resume` itself accepts + // (isValidSessionId): a single token with no newlines, escapes, or + // leading dash, which also keeps the echoed command from falling + // through to title matching on paste. Require a non-empty + // transcript too: the recorder creates the file before the first + // record lands, and `--resume` refuses to load an empty one. + // Non-emptiness here relies on config.shutdown() flushing the + // recorder first — it is registered earlier in the cleanup chain + // in gemini.tsx; keep that registration order. + if (isValidSessionId(sessionId) && (await stat(sessionFile)).size > 0) { + writeStdoutLine( + `\n${t('To continue this session, run')}\nqwen --resume ${sessionId}`, + ); + } + } + } catch { + // Best-effort: a hint must never block or break exit. + } }); }