mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-12 18:26:26 +00:00
fix(cli): echo resume command to main screen on exit (#8455)
* fix(cli): echo resume command to main screen on exit The quit screen already shows a resume hint, but in VP mode (the default) it is drawn on the alternate screen, which is discarded on teardown — users never see it. Echo the command again after Ink unmounts, when we are back on the main screen buffer, so it survives exit and can be copied from the terminal scrollback. Only printed when chat recording is enabled and the session file exists, so empty sessions do not advertise a resume that has nothing to restore. * docs(cli): add real-machine screenshots for exit resume echo * fix(cli): make exit resume echo best-effort and sanitize session ID * fix(cli): make exit resume echo tests Windows-safe and pin gates The sanitization test created a real file with ESC/BEL control bytes in its name; Win32 forbids 0x01-0x1F in file names, so the test would fail in the merge-queue Windows job. Stub the existence gate instead. Also rebuild the recording-disabled negative test on a full recording config so removing the getChatRecordingService() gate actually fails it (it previously passed via the exception path), and add coverage for the isTTY half of the gate. The source switches to a namespace fs import so the gate is stubbable in vitest (trustedFolders.ts precedent); runtime behavior is unchanged. * fix(cli): gate exit resume echo to paste-safe session IDs * fix(cli): reject dash-leading IDs in exit resume echo (#8455) A dash-leading session ID (loadable from disk: SESSION_FILE_PATTERN accepts dash-leading transcript names) pasted back as `qwen --resume -<id>` reparses as CLI flags instead of the option value, so require an alphanumeric first character in the paste-safe gate. Also switch the echo to writeStdoutLine, the helper the file already imports writeStderrLine from. * fix(cli): gate exit resume echo to canonical session IDs (#8455) Replace the inline charset gate with isValidSessionId, the same canonical predicate --resume routing, --session-id validation, and the in-TUI /resume command use. The old gate was strictly looser, so a transcript-sourced ID (e.g. via --continue) could be echoed as a hint that --resume then misroutes to title matching. The UUID shape is already paste-safe, making the extra sanitizeTerminalText wrap redundant. Also make the positive echo test locale-independent: earlier main() tests run the real initializeI18n('auto') and leave the machine locale's dictionary in the i18n module state, so the hardcoded English label failed deterministically under any supported non-English locale. Match the locale-free command part instead, and switch the echo test fixtures to valid UUIDs so each negative test isolates its intended gate. * fix(cli): align exit resume echo with --resume transcript semantics (#8455) The echo gated on fs.existsSync(sessionFile), but --resume refuses an empty transcript: ChatRecordingService creates the JSONL file before the first record lands, and a failed first write (or an exit that races the flush ceiling) leaves it empty through exit, so the hinted `qwen --resume <id>` fails with "No saved session found with ID". Require a non-empty transcript instead of switching to SessionService.sessionExists(): that check's SESSION_FILE_PATTERN rejects the -agent- suffixed Arena IDs that --resume itself accepts, which would have silently narrowed the echo's acceptance surface. Also mirror the full SESSION_ID_REGEX — including the -agent- suffix form — in the gemini.test.tsx isValidSessionId mock, which had drifted narrower than the production gate, and cover the agent-suffixed shape with a positive echo case. The now-unused node:fs existsSync mock and its stub in the hostile-ID cases are removed: the ID gate rejects before any file probe. * test(cli): Harden exit resume echo gate tests (#8455) * fix(cli): Use async stat in exit resume echo gate (#8455) The exit-cleanup chain bounds each cleanup with a 2s withTimeout race and a 5s overall ceiling, but fs.statSync() blocks the event loop, so an unresponsive filesystem (dead NFS/FUSE mount) hangs exit past both ceilings despite the surrounding try/catch. Switch to fs/promises stat — the handler is already async, so the existing race bounds it. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: qwen-code-autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
This commit is contained in:
parent
88a325bce9
commit
b9be6c8ee8
6 changed files with 252 additions and 3 deletions
BIN
.github/assets/exit-resume-echo/01-baseline-v0.21.4-blank-after-exit.png
vendored
Normal file
BIN
.github/assets/exit-resume-echo/01-baseline-v0.21.4-blank-after-exit.png
vendored
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 278 KiB |
BIN
.github/assets/exit-resume-echo/02-pr-exit-echo.png
vendored
Normal file
BIN
.github/assets/exit-resume-echo/02-pr-exit-echo.png
vendored
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 466 KiB |
BIN
.github/assets/exit-resume-echo/03-pr-resumed-session.png
vendored
Normal file
BIN
.github/assets/exit-resume-echo/03-pr-resumed-session.png
vendored
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 749 KiB |
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string>()),
|
||||
// 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<void> {
|
||||
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> | 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();
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue