From f220d676367680fd04dc9dc0f1b7ad96a8a75977 Mon Sep 17 00:00:00 2001 From: mac Date: Mon, 7 Sep 2026 02:25:42 +0800 Subject: [PATCH] fix(cli): fence the sessions-answer payload off the help/version intercepts A free-text answer to `qwen sessions answer ` that quotes `--help`/`--version` (or ends in a bare `help`) was swallowed by the bootstrap scans and the root parser's help/version handling: the CLI printed the usage block or the version and exited 0, so a script driving a background session read success while the answer was never delivered. Insert a single `--` after the session id, in both the bootstrap route scan and parseArguments' raw argv, so every later token is positional data to those scans and to the yargs parse. The carve-outs stay: a bare `--help` payload (and `sessions answer --help` with no id) keeps showing the command's help, a user-supplied separator is never doubled, and the fail-closed version intercept for every other command is untouched. Fixes #11193 --- packages/cli/src/cli.test.ts | 72 ++++++++ packages/cli/src/cli.ts | 12 +- packages/cli/src/config/config.test.ts | 59 +++++++ packages/cli/src/config/config.ts | 10 ++ .../cli/src/utils/session-answer-argv.test.ts | 161 ++++++++++++++++++ packages/cli/src/utils/session-answer-argv.ts | 67 ++++++++ 6 files changed, 380 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/utils/session-answer-argv.test.ts create mode 100644 packages/cli/src/utils/session-answer-argv.ts diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index f76525fe5d9..0359fe1424d 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -127,6 +127,78 @@ describe('resolveBootstrapRoute', () => { expect(resolveBootstrapRoute(['mcp', '--help'])).toBe('mcp'); }); + it('does not let the answer payload trigger a bootstrap intercept', () => { + // The free-text tail of `sessions answer ` must not reach the + // interceptors as flag tokens: each of these printed help/version and + // exited 0, telling the driving script the answer was delivered when + // it was not (issue #11193). The separator fences the payload off, so + // the argv routes to the full parser instead. + expect( + resolveBootstrapRoute([ + 'sessions', + 'answer', + '0f8e1c42', + 'please', + '--help', + 'me', + ]), + ).toBe('default'); + expect( + resolveBootstrapRoute([ + 'sessions', + 'answer', + '0f8e1c42', + 'yes', + 'please', + 'help', + ]), + ).toBe('default'); + expect( + resolveBootstrapRoute([ + 'sessions', + 'answer', + '0f8e1c42', + 'please', + '--version', + 'now', + ]), + ).toBe('default'); + // A user-supplied separator already fenced the payload; the route must + // not change (and the separator must not be doubled). + expect( + resolveBootstrapRoute([ + 'sessions', + 'answer', + '0f8e1c42', + '--', + 'please', + '--version', + 'now', + ]), + ).toBe('default'); + }); + + it('keeps the intercepts for every other command and shape', () => { + // The fence is scoped to the answer payload: version tokens elsewhere + // keep the fail-closed intercept (issue #11193's guardrail — demoting + // those executes subcommands). + expect(resolveBootstrapRoute(['sessions', 'list', '-v'])).toBe('version'); + expect(resolveBootstrapRoute(['sessions', 'ps', '--version'])).toBe( + 'version', + ); + expect( + resolveBootstrapRoute(['mcp', 'remove', 'victim', '-v', 'help']), + ).toBe('version'); + // `qwen sessions answer --help` (no id) and a bare `--help` payload + // keep routing to the parser, which shows the command's help. + expect(resolveBootstrapRoute(['sessions', 'answer', '--help'])).toBe( + 'default', + ); + expect( + resolveBootstrapRoute(['sessions', 'answer', '0f8e1c42', '--help']), + ).toBe('default'); + }); + it('keeps bundled entrypoint paths out of the route detection', async () => { expect(resolveBootstrapRoute(['/repo/dist/cli.js', '--help'])).toBe('help'); expect( diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index bb5937c7f1d..368257d407e 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -24,6 +24,7 @@ import { } from './config/top-level-options.js'; import { clearInheritedPeerMessagingEnv } from './peerMessaging/env.js'; import { normalizeServeFastPathArgv } from './utils/serve-fast-path-argv.js'; +import { insertSessionAnswerSeparator } from './utils/session-answer-argv.js'; import { initStartupProfiler } from './utils/startupProfiler.js'; import { initCpuProfiler } from './utils/cpuProfiler.js'; import { @@ -322,7 +323,16 @@ function normalizeMcpFastPathArgv(argv: readonly string[]): readonly string[] { export function resolveBootstrapRoute( rawArgv: readonly string[], ): BootstrapRoute { - const argv = normalizeServeFastPathArgv(rawArgv); + // Fence the `sessions answer ` payload off from + // the scans below before they run: an answer that quotes `--help`, + // `--version` or ends in a bare `help` would otherwise be swallowed by + // an intercept that prints help/version and exits 0, telling the driving + // script the answer was delivered when it was not (issue #11193). After + // the separator every later token is positional data to these scans by + // construction, so no intercept fires on the payload. + const argv = insertSessionAnswerSeparator( + normalizeServeFastPathArgv(rawArgv), + ); // Base-parity version intercept (structural close). Base printed the // version for any `-v`/`--version` token its hasFlag scan reached, no diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 71c0059c6fd..ea676ff04f7 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -318,6 +318,65 @@ describe('parseArguments', () => { } }); + it('does not let a sessions-answer payload quote help into the help intercept', async () => { + // Issue #11193: a free-text answer that quotes `--help` (or ends in a + // bare `help`) made the root parser print the usage block and exit 0, + // so the driving script read success while the answer was never + // delivered. The separator fences the payload off, so the parse takes + // the strict unknown-command failure instead — loud, non-zero. (The + // yargs parser holds the test runner's process.exit stub, so the + // rejection message itself carries the exit code it asked for.) + const swallowArgv = [ + 'sessions', + 'answer', + '0f8e1c42', + 'please', + '--help', + 'me', + ]; + const bareHelpArgv = [ + 'sessions', + 'answer', + '0f8e1c42', + 'yes', + 'please', + 'help', + ]; + for (const answerArgv of [swallowArgv, bareHelpArgv]) { + process.argv = ['node', 'script.js', ...answerArgv]; + await expect(parseArguments()).rejects.toThrow( + 'process.exit unexpectedly called with "1"', + ); + } + }); + + it('keeps a bare --help answer showing help rather than delivering it', async () => { + // The carve-out the answer command's positional documents: a payload + // that is exactly `--help` keeps reaching the parser as the help + // flag, so it still shows help (exit 0) instead of answering with the + // literal text `--help`. + process.argv = [ + 'node', + 'script.js', + 'sessions', + 'answer', + '0f8e1c42', + '--help', + ]; + const output: string[] = []; + const log = vi.spyOn(console, 'log').mockImplementation((...args) => { + output.push(args.join(' ')); + }); + try { + await expect(parseArguments()).rejects.toThrow( + 'process.exit unexpectedly called with "0"', + ); + expect(output.join('')).toContain('qwen sessions'); + } finally { + log.mockRestore(); + } + }); + it('should throw an error when both --prompt and --prompt-interactive are used together', async () => { process.argv = [ 'node', diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index d1e65a738b4..858f21f52d2 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -71,6 +71,7 @@ import { randomUUID } from 'node:crypto'; import stripJsonComments from 'strip-json-comments'; import { resolvePath } from '../utils/resolvePath.js'; +import { insertSessionAnswerSeparator } from '../utils/session-answer-argv.js'; import { TOP_LEVEL_GLOBAL_OPTIONS, DEFAULT_COMMAND, @@ -579,6 +580,15 @@ export async function parseArguments(): Promise { rawArgv = rawArgv.slice(1); } + // Fence the free-text answer payload off from the root parse: a quoted + // `--help`/`--version` (or a bare trailing `help`) inside `qwen sessions + // answer ` would otherwise print the usage block or + // strip the token from the text and exit 0, so the answer is silently + // never delivered (issue #11193). After the separator the payload is + // positional data to this parse; the answer command splices the first + // `--` back out of its raw-args tail, so the text arrives verbatim. + rawArgv = insertSessionAnswerSeparator(rawArgv); + const yargsInstance = yargs(rawArgv) .locale('en') .scriptName('qwen') diff --git a/packages/cli/src/utils/session-answer-argv.test.ts b/packages/cli/src/utils/session-answer-argv.test.ts new file mode 100644 index 00000000000..698a17fa3f7 --- /dev/null +++ b/packages/cli/src/utils/session-answer-argv.test.ts @@ -0,0 +1,161 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { insertSessionAnswerSeparator } from './session-answer-argv.js'; + +describe('insertSessionAnswerSeparator', () => { + it('inserts -- between the session id and a free-text payload', () => { + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + 'yes, go ahead', + ]), + ).toEqual(['sessions', 'answer', '0f8e1c42', '--', 'yes, go ahead']); + }); + + it('fences a payload that quotes help/version tokens', () => { + // The three shapes from issue #11193's reproduction table. + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + 'please', + '--help', + 'me', + ]), + ).toEqual([ + 'sessions', + 'answer', + '0f8e1c42', + '--', + 'please', + '--help', + 'me', + ]); + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + 'yes', + 'please', + 'help', + ]), + ).toEqual([ + 'sessions', + 'answer', + '0f8e1c42', + '--', + 'yes', + 'please', + 'help', + ]); + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + 'please', + '--version', + 'now', + ]), + ).toEqual([ + 'sessions', + 'answer', + '0f8e1c42', + '--', + 'please', + '--version', + 'now', + ]); + }); + + it('does not touch other commands or unprefixed argv', () => { + const untouched: string[][] = [ + ['--help'], + ['--version'], + ['sessions', 'list'], + ['sessions', 'ps', '--json'], + ['mcp', 'remove', 'victim', '-v', 'help'], + ['peek', '0f8e1c42'], + ['answer', '0f8e1c42', 'yes'], + ]; + for (const argv of untouched) { + expect(insertSessionAnswerSeparator(argv)).toEqual(argv); + } + }); + + it('keeps a missing or flag-like session id for the parser to reject', () => { + expect(insertSessionAnswerSeparator(['sessions', 'answer'])).toEqual([ + 'sessions', + 'answer', + ]); + // `qwen sessions answer --help` (no id) still routes to the command's + // help; the demandOption error for a missing id is yargs' to raise. + expect( + insertSessionAnswerSeparator(['sessions', 'answer', '--help']), + ).toEqual(['sessions', 'answer', '--help']); + expect(insertSessionAnswerSeparator(['sessions', 'answer', '-v'])).toEqual([ + 'sessions', + 'answer', + '-v', + ]); + }); + + it('keeps a bare --help payload showing help (the documented carve-out)', () => { + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + '--help', + ]), + ).toEqual(['sessions', 'answer', '0f8e1c42', '--help']); + expect( + insertSessionAnswerSeparator(['sessions', 'answer', '0f8e1c42', '-h']), + ).toEqual(['sessions', 'answer', '0f8e1c42', '-h']); + // Only the bare form: `--help` with a sibling token is answer text. + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + '--help', + 'me', + ]), + ).toEqual(['sessions', 'answer', '0f8e1c42', '--', '--help', 'me']); + }); + + it('does not double a user-supplied separator', () => { + expect( + insertSessionAnswerSeparator([ + 'sessions', + 'answer', + '0f8e1c42', + '--', + '--force', + ]), + ).toEqual(['sessions', 'answer', '0f8e1c42', '--', '--force']); + }); + + it('leaves an id with no payload alone', () => { + expect( + insertSessionAnswerSeparator(['sessions', 'answer', '0f8e1c42']), + ).toEqual(['sessions', 'answer', '0f8e1c42']); + }); + + it('returns a new array only when it inserts', () => { + const untouched = ['sessions', 'answer', '0f8e1c42']; + expect(insertSessionAnswerSeparator(untouched)).toBe(untouched); + const fenced = insertSessionAnswerSeparator([...untouched, 'yes']); + expect(fenced).not.toBe(untouched); + expect(untouched).toEqual(['sessions', 'answer', '0f8e1c42']); + }); +}); diff --git a/packages/cli/src/utils/session-answer-argv.ts b/packages/cli/src/utils/session-answer-argv.ts new file mode 100644 index 00000000000..2747a31e0ec --- /dev/null +++ b/packages/cli/src/utils/session-answer-argv.ts @@ -0,0 +1,67 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Fence off the free-text payload of `qwen sessions answer ` from + * every flag scan that runs before and during the yargs parse. + * + * `sessions answer` is the one command whose positional tail is arbitrary + * user prose, so a token like `--help`, `--version` or the bare word + * `help` inside the answer is indistinguishable from the flag it spells. + * Three interceptors act on exactly that spelling before any command + * handler can see it: the bootstrap version scan (`hasVersionToken` in + * cli.ts) prints the version and exits 0, and the root parser's + * `.help()`/`.version()` registrations (config.ts) print the usage block + * or strip the token out of the text — all with exit 0, so a script + * driving a background session reads success while the answer was never + * delivered. + * + * Inserting a single `--` right after the session id closes every one of + * those paths at once: tokens after `--` are positional data to yargs and + * to both bootstrap scans, which stop at the separator by construction. + * The command's own tail recovery (the raw-args tail in the `answer` + * command) splices the first `--` back out, so the delivered text is the + * argv verbatim — the separator is scaffolding, not payload. + * + * The match is a strict argv prefix rather than a positional scan: `sessions` + * as the first token can only be the command (nothing before it can hold a + * value slot), so there is no value-slot model to get wrong here. A + * flag-shifted shape (`qwen --debug sessions answer …`) is left to the argv + * scan consolidation that issue #11065 owns. + * + * Carve-outs that keep the command's documented surface intact: + * - `` must be a real positional. `qwen sessions answer --help` + * (no id) keeps routing to the command's help, and a missing id is the + * parser's demandOption error to raise, not ours. + * - A payload that is exactly `--help` (or `-h`) keeps showing the + * command's help — the bare-`--help` carve-out the positional's describe + * and control-commands tests promise. + * - A user-supplied separator is never doubled: the tail recovery splices + * only the first `--`, so a second one would leak into the answer text. + */ +export function insertSessionAnswerSeparator( + argv: readonly string[], +): string[] { + if (argv[0] !== 'sessions' || argv[1] !== 'answer') { + return argv as string[]; + } + const session = argv[2]; + if (session === undefined || session.startsWith('-')) { + return argv as string[]; + } + // No payload after the id: nothing to fence off. + if (argv.length < 4) { + return argv as string[]; + } + if (argv[3] === '--') { + return argv as string[]; + } + // The bare-`--help` carve-out: `answer --help` shows help. + if (argv.length === 4 && (argv[3] === '--help' || argv[3] === '-h')) { + return argv as string[]; + } + return [...argv.slice(0, 3), '--', ...argv.slice(3)]; +}