[Suggestion] No test coverage for the getListExtensions() === true path. All existing test mocks in gemini.test.tsx use getListExtensions: () => false, so this new handler is untested. Consider adding a test that exercises the true branch — at minimum verifying the empty-extension case ("No extensions installed.") and the non-empty case with at least one disabled extension (to cover the [disabled] suffix).

— qwen-latest-series-invite-beta-v38 via Qwen Code /review

Good catch — added two test cases in 1076d6c:

1. Empty extensions — verifies "No extensions installed." is printed and process.exit(0) is called
2. Mixed active/disabled extensions — verifies the listing format including the [disabled] suffix for inactive extensions

Both follow the existing mock patterns in the test file.

[Critical] config.getExtensions() is called BEFORE config.initialize() (line ~836). getExtensions() delegates to ExtensionManager.getLoadedExtensions(), which returns [] when extensionCache is null — its initial state. The cache is only populated during config.initialize() via extensionManager.refreshCache(). The original FIXME comment explicitly warned: "FIXME: list extensions after the config initialize".

Impact: --list-extensions always prints "No extensions installed." and exits 0, regardless of what is actually installed. 100% reproducible. Tests miss this because they mock config.getExtensions() at the return-value level, bypassing the real initialization path.

Fix: Move this entire block to after await config.initialize() (around line 855), and add await runExitCleanup() before process.exit(0) to match other post-init early exits. Alternatively:

if (config.getListExtensions()) {
  await config.initialize();
  const extensions = config.getExtensions();
  // ...existing formatting...
  await runExitCleanup();
  process.exit(0);
}

— qwen3.7-max via Qwen Code /review

[Suggestion] preconnectApi() fires a network HEAD request (line ~662) before this early-exit handler. When --list-extensions is set, one HEAD request is wasted then immediately torn down by process.exit(0). On slow or unreachable networks, this adds latency to what should be an instant command.

Consider moving this handler before preconnectApi() to avoid unnecessary network I/O, or at minimum ensure the Critical ordering fix (moving after config.initialize()) also accounts for this.

— qwen3.7-max via Qwen Code /review

[Critical] eslint no-console violation. The codebase convention requires // eslint-disable-next-line no-console with a justification for every console.* call (see 7 examples in packages/cli/src/).

— qwen3.7-max via Qwen Code /review

[Critical] eslint no-console violation.

— qwen3.7-max via Qwen Code /review

[Critical] eslint no-console violation.

— qwen3.7-max via Qwen Code /review

[Suggestion] Both new tests mock getExtensions() to return hardcoded arrays (() => [] and () => [{ ... }]), bypassing the real Config class and ExtensionManager entirely. This means the tests pass regardless of whether config.initialize() is called before getExtensions() — the critical initialization ordering bug (open comment at gemini.tsx:676) is structurally invisible to these tests.

Consider adding an assertion that initialize() (or extensionManager.refreshCache()) was called before getExtensions(), or an integration test that exercises the real ExtensionManager with a seeded extensions directory. Without that, the tests verify output formatting but not data correctness — the exact bug that breaks production is the one thing they cannot catch.

— qwen3.7-max via Qwen Code /review

[Suggestion] process.exit(0) is called without await runExitCleanup(). registerCleanup(() => config.shutdown()) was registered at line ~658, and every other exit path after that point (lines 749, 792, 913, 955) consistently calls await runExitCleanup() before process.exit(). This is the only handler that skips it.

While config.shutdown() is effectively a no-op before initialize() runs, this breaks the cleanup invariant and will silently leak resources if any future code registers a pre-initialization cleanup handler.

— qwen3.7-max via Qwen Code /review

[Critical] initializeApp() ≠ config.initialize() — the handler is placed after initializeApp() (line 725) but config.initialize() is at line 841, over 100 lines below. Extensions are only loaded when config.initialize() calls extensionManager.refreshCache() (see config.ts:1188 → extensionManager.ts:556). Before that call, getLoadedExtensions() returns [] because extensionCache is null (extensionManager.ts:561-566).

The result: --list-extensions always prints "No extensions installed." regardless of what is actually installed. The feature remains 100% non-functional. Tests pass because they mock getExtensions() directly, bypassing the real extension-loading pipeline.

The original FIXME said "list extensions after the config initialize" — meaning config.initialize(), not initializeApp().

Fix: Move this block to after await config.initialize() (line 841):

// After line 842 (after profileCheckpoint('config_initialize_end')):
if (config.getListExtensions()) {
  const extensions = config.getExtensions();
  // ... existing display logic unchanged ...
  await runExitCleanup();
  process.exit(0);
}

— qwen3.7-max via Qwen Code /review

[Critical] initializeWarningHandler() was removed from main() without mention in the PR description. This function (warningHandler.ts:65) installs a process.on('warning', ...) listener that suppresses spurious MaxListenersExceededWarning for AbortSignal — a structural condition in long-running agent sessions with many tool rounds. Without it, users will see raw Node.js internal warnings on stderr during normal operation.

The warningHandler.ts module and its test file still exist but are now dead code — no production caller remains.

This removal is unrelated to --list-extensions. Please restore the call, or if intentional, move it to a separate PR with rationale and delete the now-dead warningHandler.ts / warningHandler.test.ts.

— qwen3.7-max via Qwen Code /review

[Critical] The headless YOLO safety warning was silently removed. When running qwen -p "prompt" --yolo without a sandbox, users no longer receive any stderr signal that all tool calls auto-execute at the process's privilege level. This was a deliberate safety feature (issue #4103) — the function getHeadlessYoloSafetyWarning() still exists and is used in serve.ts, confirming it was not deprecated.

This removal is undocumented in the PR description and unrelated to --list-extensions. Please restore the warning block, or explain the rationale for removing it.

— qwen3.7-max via Qwen Code /review

[Critical] [typecheck] maxWallTime and maxToolCalls were removed from this parseArguments mock, but CliArgs (config.ts:179-180) still declares them as required fields. This causes tsc --noEmit to fail:

gemini.test.tsx(881,49): error TS2345: Argument of type '{ model: undefined; ... }' is not assignable to parameter of type 'CliArgs'.
  Type '...' is missing the following properties from type 'CliArgs': maxWallTime, maxToolCalls

Restore the two fields:

— qwen3.7-max via Qwen Code /review

[Suggestion] The comment states "The child starts with a clean settings file" but this is inaccurate. The child process is a relaunch of the same binary in the same directory — it calls loadSettings() on the same file. The actual reason migration warnings need pre-relaunch emission is that the parent exits inside relaunchAppInChildProcess() before reaching the main startupWarnings display path (line ~771). The child's own loadSettings() would independently re-derive and display them.

Consider rewriting for accuracy:

— qwen3.7-max via Qwen Code /review

[Suggestion] extension.version is interpolated directly into console.log output without validation. Extension names are validated by validateName() (^[a-zA-Z0-9-_.]+$), but versions have no equivalent check — a malicious extension could set its version to a string containing ANSI escape sequences (e.g., \u001b[2K\u001b[A) to manipulate terminal output or inject clipboard content via OSC 52.

While installed extensions are implicitly trusted to some degree, defense-in-depth suggests validating the version field (e.g., a semver-like regex or stripping non-printable characters) before output.

— qwen3.7-max via Qwen Code /review

[Suggestion] initializeApp() performs unnecessary work for the --list-extensions early-exit path. It calls initializeI18n() (loads translation files), performInitialAuth() (may refresh OAuth tokens via network call), and validateTheme() — none of which are needed to list installed extensions.

When fixing the init ordering issue (moving the handler after config.initialize() or calling extensionManager.refreshCache() directly), consider restructuring so --list-extensions avoids the full initialization overhead. For example, call extensionManager.refreshCache() directly instead of going through the full config.initialize() + initializeApp() pipeline:

if (config.getListExtensions()) {
  // Only load extension data — skip full init (MCP, hooks, auth)
  await config.getExtensionManager().refreshCache();
  const extensions = config.getExtensions();
  // ... rest of handler
}

This would make --list-extensions a fast, lightweight command that doesn't pay the cost of auth, i18n, or MCP initialization.

— qwen3.7-max via Qwen Code /review

[Critical] --list-extensions is unreachable from interactive (TTY) mode — the most common use case.

This handler is inside the non-interactive branch (after the if (config.isInteractive()) block at ~line 921). When a user runs qwen --list-extensions in a terminal, config.isInteractive() returns true, routing into startInteractiveUI() which returns before reaching this code. The flag is silently ignored and the normal TUI launches instead.

The --json-schema handler at line ~930 shows the correct pattern — check for the flag inside the interactive branch and handle it before startInteractiveUI():

if (config.isInteractive()) {
  if (config.getListExtensions()) {
    await config.initialize();
    const extensions = config.getExtensions();
    if (extensions.length === 0) {
      console.log('No extensions installed.');
    } else {
      console.log('Installed extensions:');
      for (const extension of extensions) {
        const safeVersion = extension.version.replace(/[\x00-\x1f\x7f-\x9f]/g, '');
        console.log(`- ${extension.name} (v${safeVersion})${extension.isActive ? '' : ' [disabled]'}`);
      }
    }
    await runExitCleanup();
    process.exit(0);
  }
  // ... existing json-schema check, startInteractiveUI, etc.
}

Note: both new tests mock isInteractive: () => false, so this TTY path is untested.

— qwen3.7-max via Qwen Code /review

[Suggestion] --list-extensions is silently ignored when combined with --input-format stream-json. The handler is inside if (inputFormat !== InputFormat.STREAM_JSON), so qwen --list-extensions --input-format stream-json proceeds into session startup with no warning.

Consider adding a conflict check in the yargs .check() validation or extracting the handler above the input-format guard so it always runs regardless of input format.

— qwen3.7-max via Qwen Code /review

[Suggestion] If config.initialize() throws (e.g., corrupted extension JSON, disk I/O error), this handler is never reached and runExitCleanup() is skipped — registered cleanup callbacks (including config.shutdown() at line ~808) don't run, potentially leaving MCP server subprocesses orphaned.

Consider wrapping the handler in a try/catch that reports a user-friendly error and guarantees cleanup:

if (config.getListExtensions()) {
  try {
    const extensions = config.getExtensions();
    // ... print logic ...
  } catch (error) {
    writeStderrLine(`Error: Failed to list extensions — ${error instanceof Error ? error.message : String(error)}`);
  } finally {
    await runExitCleanup();
    process.exit(0);
  }
}

— qwen3.7-max via Qwen Code /review

[Critical] config.initialize() is gated behind if (inputFormat === InputFormat.STREAM_JSON). For the default TEXT input format (the common case for qwen --list-extensions in a terminal), config.initialize() is never called before config.getExtensions(). Extensions are loaded via extensionManager.refreshCache() inside config.initialize(), so getExtensions() returns [] regardless of what is installed — the flag always prints "No extensions installed."

Both new tests fail with expected "spy" to be called 1 times, but got 0 times, confirming the bug. This is the same init-ordering Critical from prior review rounds — the fix is still incomplete.

— qwen3.7-max via Qwen Code /review

[Suggestion] extension.name is interpolated raw into console.log output while extension.version on the same line is sanitized via /[\x00-\x1f\x7f-\x9f]/g. A malicious extension with ANSI escape sequences in its name could inject terminal control codes (cursor repositioning, OSC 8 hyperlinks, etc.). Apply the same sanitization:

— qwen3.7-max via Qwen Code /review

[Suggestion] mock.results[0]! retrieves the config mock from the first test, not the current one. vi.restoreAllMocks() restores spy implementations but does not clear accumulated mock.results for vi.mock() module mocks. Use mock.results.at(-1) to get the most recent call's result:

— qwen3.7-max via Qwen Code /review

[Suggestion] When --list-extensions runs on a TTY, the interactive startup block sets setRawMode(true) (line 836) and starts early input capture. process.exit(0) here does not restore raw mode — runExitCleanup() runs installInteractiveSignalHandlers cleanup which only removes signal handlers, not termios state. The terminal is left in raw mode until the shell resets it. Add explicit restoration:

— qwen3.7-max via Qwen Code /review

[Suggestion] Migration warnings are emitted twice in the parent process. This new loop duplicates the pre-existing emission at line 436-441 via getSettingsWarnings(), which already iterates loadedSettings.migrationWarnings with the "Warning: " prefix (see settings.ts:335-337). Both blocks execute in the parent before relaunchAppInChildProcess() is called.

The comment claims "Without this early emit, migration warnings would be silently lost" — but line 436-441 already solves this exact problem, with its own comment: "Emit settings warnings early so the parent process surfaces them before relaunchAppInChildProcess() exits."

Fix: Remove this block (lines 595-602, including the comment). The pre-existing early emission at line 436-441 is sufficient.

— qwen3.7-max via Qwen Code /review

nit (placement): this --list-extensions short-circuit sits after the raw-mode + Kitty/theme detection setup (lines ~687–738).

For the common invocation qwen --list-extensions in a TTY, isInteractive() is true (no -p/query + TTY → interactive, see packages/cli/src/config/config.ts:1428), so the CLI runs process.stdin.setRawMode(true) and fires the Kitty-protocol / OSC-11 theme probes before listing and exiting. Raw mode itself is correctly restored via runExitCleanup() (the registered installInteractiveSignalHandlers cleanup), but kittyProtocolDetectionComplete is started and never awaited on this path, so on terminals that don't speak the Kitty protocol a stray detection response can leak into the shell after exit.

Consider moving this block earlier — right after loadCliConfig() / before the raw-mode + detection setup. That keeps "list and exit" purely synchronous and would also let you drop the if (!config.getListExtensions()) guard added around preconnectApi (line ~809), since main() would already have returned. Not blocking.

nit: this is a whitespace/indentation-only change to the <AppContainer> JSX, unrelated to the --list-extensions fix. Worth reverting to keep the diff focused and avoid churning git blame on lines this PR doesn't functionally touch.

[Suggestion] When config.initialize() throws, the error is caught and logged only to debugLogger.warn (invisible without --debug). Execution falls through to getExtensions() which returns [] because extensionCache was never populated. The user sees "No extensions installed." with exit code 0 — factually incorrect output if extensions are installed but initialization failed (e.g., git unavailable, corrupt hooks config, disk I/O error).

Consider surfacing the failure visibly:

— qwen3.7-max via Qwen Code /review

[Suggestion] Both tests mock initialize to resolve successfully, so the config.initialize() failure path is entirely untested. Combined with the silent-error-handling issue in gemini.tsx, this means the failure behavior (misleading "No extensions installed." + exit 0) has no test coverage.

Consider adding a test where initialize rejects, and also asserting runExitCleanupMock and the initialize mock were called in existing tests — currently neither is verified, so removing those calls wouldn't cause test failures.

— qwen3.7-max via Qwen Code /review