Code Review

This pull request modifies the mechanism for passing additional Node.js arguments during process relaunch. Instead of appending arguments like memory settings directly to the command-line array, they are now injected into the NODE_OPTIONS environment variable, ensuring existing options are preserved. These changes are implemented in the CLI entry point and the relaunch utility, with comprehensive updates to the test suite to verify the correct environment configuration. I have no feedback to provide as no review comments were submitted.

Code Review

This pull request updates the CLI relaunch logic to support a standalone binary (SEA) mode by moving Node.js flags into the NODE_OPTIONS environment variable, while maintaining standard command-line argument passing for non-binary execution. Corresponding updates were made to the test suite to validate both modes. I have identified a potential issue where joining arguments with a space in NODE_OPTIONS will fail if any argument contains a space; I have provided a suggestion to escape these spaces correctly.

Code Review

This pull request implements support for Single Executable Application (SEA) mode by transitioning Node.js flags to the NODE_OPTIONS environment variable when the IS_BINARY flag is enabled. Review feedback identifies that the current escaping logic for spaces in NODE_OPTIONS using backslashes is incorrect for the Node.js runtime and should be replaced with double quotes. Additionally, the test suite requires refactoring to avoid re-implementing production logic within test cases, which currently results in fragile change-detector tests and inconsistencies between the test simulations and the actual implementation.

Code Review

This pull request introduces support for Single Executable Application (SEA) mode by moving Node.js arguments into the NODE_OPTIONS environment variable when the IS_BINARY environment variable is set. This logic is implemented in both the main CLI entry point and the relaunch utility, with corresponding updates to the test suite. Review feedback highlighted a bug in the NODE_OPTIONS escaping logic where backslashes are incorrectly escaped in unquoted strings, which could break Windows paths. Additionally, it was recommended to extract this duplicated escaping logic into a shared utility to improve maintainability.

Code Review

This pull request introduces support for Single Executable Application (SEA) mode by routing Node.js flags through the NODE_OPTIONS environment variable when the IS_BINARY flag is set. Review feedback identifies a bug in the formatNodeOptions utility, which incorrectly doubles backslashes that the Node.js runtime does not unescape. Additionally, the reviewer recommends encapsulating duplicated relaunch logic found in both index.ts and relaunch.ts, replacing hardcoded exit codes with imported constants, and correcting test expectations to ensure proper argument formatting.

Code Review

This pull request refactors the CLI's process relaunch mechanism by introducing a centralized getSpawnConfig utility that handles environment and argument construction for both standard Node.js and Single Executable Application (SEA) modes. It also includes a new formatNodeOptions helper for escaping arguments within the NODE_OPTIONS environment variable. Review feedback identified a bug in formatNodeOptions where trailing backslashes could break quote escaping and suggested avoiding the duplication of inherited flags in SEA mode to prevent exceeding environment variable length limits.

Code Review

This pull request refactors the process relaunch logic to support Single Executable Application (SEA) binary mode. It introduces getSpawnConfig and formatNodeOptions in processUtils.ts to handle argument construction and environment variable configuration differently for standard Node and binary modes. In binary mode, node arguments are now passed via the NODE_OPTIONS environment variable with proper escaping and quoting, while standard mode continues to use command-line arguments. I have no feedback to provide.

Code Review

This pull request introduces a getSpawnConfig utility to handle process relaunching logic, specifically addressing differences between standard Node.js execution and Single Executable Application (SEA) binary mode. The changes refactor the argument construction in relaunchAppInChildProcess to use this new utility and include updated tests. The reviewer highlighted concerns regarding the fragility of joining nodeArgs with spaces in NODE_OPTIONS and the reliability of using an environment variable (IS_BINARY) for environment detection, suggesting more robust alternatives.

Code Review

This pull request refactors the process relaunch logic by introducing a centralized getSpawnConfig utility to handle both standard Node.js and Single Executable Application (SEA) binary modes. The changes ensure that Node arguments are correctly passed via NODE_OPTIONS in SEA mode. Feedback was provided to strengthen the validation of unsupported characters in nodeArgs to prevent injection and to adhere to the project's style guide by using vi.stubEnv instead of direct process.env modification in tests.

Code Review

This pull request refactors the process relaunch logic into a centralized getSpawnConfig utility to support Single Executable Application (SEA) binary modes. It also updates the test suite to use Vitest's environment stubbing utilities as per the style guide. Review feedback highlights the need for more robust SEA detection using process.isSea(), maintaining a consistent argv structure (preserving the slice(2) expectation) when spawning child processes in binary mode, and correctly handling argument slicing in the main entry point for standard SEA binaries.

Code Review

This pull request introduces support for Single Executable Application (SEA) binary mode by centralizing process spawning logic into a new getSpawnConfig utility. In SEA mode, Node-specific arguments are now correctly passed via the NODE_OPTIONS environment variable to ensure they are processed by the runtime. Additionally, the test suite has been refactored to adhere to the repository style guide by using vi.stubEnv and vi.unstubAllEnvs for environment variable management. I have no feedback to provide as there were no review comments to assess.

from /review-frontend

Review Summary

The PR successfully addresses the issue by appropriately routing Node arguments through the NODE_OPTIONS environment variable when running in SEA (Single Executable Application) mode. The implementation also includes solid validation to prevent injection vulnerabilities by blocking complex arguments with spaces or quotes.

However, there is a critical logic flaw in how script arguments are passed to the child process that will cause compounding argument duplication upon consecutive relaunches.

Findings

Critical: Compounding Argument Duplication

The interaction between isStandardSea and the script placeholder in getSpawnConfig can create a compounding argument duplication bug if the app is relaunched more than once (e.g., a restart triggered from within the application).

The Scenario:

1. A user runs gemini my-cmd. process.argv is ['/bin/gemini', 'my-cmd'].
2. isStandardSea evaluates to true, so scriptArgs is parsed as ['my-cmd'].
3. In getSpawnConfig, it uses process.argv[1] ?? process.execPath as the placeholder. Because process.argv[1] is 'my-cmd', finalSpawnArgs becomes ['my-cmd', 'my-cmd'].
4. The spawned child process receives process.argv = ['/bin/gemini', 'my-cmd', 'my-cmd']. The CLI parser works fine here because it ignores the first two elements (slice(2)).
5. The Bug: If that child process needs to relaunch again, it will check argv[0] !== argv[1] ('/bin/gemini' !== 'my-cmd'), which means isStandardSea evaluates to true again! It will incorrectly slice from index 1, capturing ['my-cmd', 'my-cmd'] as the scriptArgs, and pass those forward. The arguments will compound indefinitely on each relaunch.

Suggested Fixes:

1. In packages/cli/src/utils/processUtils.ts, always use process.execPath as the explicit placeholder instead of dynamically using process.argv[1]. This guarantees argv[0] === argv[1] in the child process, breaking the cycle.

   // Change:
   // finalSpawnArgs.push(process.argv[1] ?? process.execPath, ...scriptArgs);
   
   // To:
   finalSpawnArgs.push(process.execPath, ...scriptArgs);

2. In index.ts and relaunch.ts, group the argv[0] !== argv[1] check so that it applies to both IS_BINARY and isSea(). This ensures the child properly identifies the injected placeholder even if Node native process.isSea() is true.

   const isStandardSea =
     process.argv[0] !== process.argv[1] &&
     (process.env['IS_BINARY'] === 'true' || (process as ProcessWithSea).isSea?.() === true);

Improvement: DRY up SEA Detection

The ProcessWithSea interface and the SEA detection logic (isStandardSea / isBinary) are currently duplicated across index.ts, relaunch.ts, and processUtils.ts. Consider extracting a shared helper function (e.g., getScriptArgs and isStandardSea) into processUtils.ts to consolidate this logic.

Conclusion

Status: Request Changes
The core strategy of utilizing NODE_OPTIONS is solid, but the script argument passing logic needs to be corrected to prevent the compounding argument bug.

minor nit but approving