fix(cli): scrub CI env vars in dev mode to prevent interactive hang#25260
fix(cli): scrub CI env vars in dev mode to prevent interactive hang#25260KeWang0622 wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
The bundled build patches `is-in-ci` via an esbuild alias so that `ink` stays in interactive mode regardless of CI env vars. However, `npm run start` (the dev server) bypasses esbuild and runs TypeScript directly, so the alias never applies. When `CI` or `CONTINUOUS_INTEGRATION` is set in the shell (common for contributors working in CI-adjacent environments), `ink` switches to non-interactive mode and the CLI hangs after the ASCII art header. Extract a `scrubCiEnv` utility that strips these vars from the child process environment, print a warning to stderr when scrubbing occurs, and add 7 unit tests covering the truthy/falsy/absent cases. Fixes google-gemini#22452
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the development server would hang indefinitely when running in environments where CI-related environment variables are set. By stripping these variables before spawning the child process, the CLI is prevented from incorrectly switching to non-interactive mode, thereby restoring expected functionality for contributors. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a utility to scrub CI-related environment variables when running the development server to ensure the terminal remains interactive. A review comment points out that the environment variables should be deleted even if they are set to "0" or "false" because the mere presence of these keys can still trigger non-interactive mode in some libraries.
| if (key in env && env[key] !== '0' && env[key] !== 'false') { | ||
| scrubbed.push(key); | ||
| delete env[key]; | ||
| } |
There was a problem hiding this comment.
The logic to preserve '0' and 'false' values is likely incorrect and will not prevent the interactive hang in most environments.
Most common implementations of is-in-ci (including the one used by ink via is-interactive) check for the existence of the key ('CI' in env) or its truthiness as a string (!!env.CI). In Node.js, process.env values are always strings, and both '0' and 'false' are truthy strings. Consequently, if these variables are left in the environment, is-in-ci will still return true, and the CLI will continue to hang.
To ensure interactive mode is enabled while still allowing users to suppress the warning as intended by the message in start.js, the variables should be removed from the environment regardless of their value, but only added to the scrubbed list if they are not '0' or 'false'.
if (key in env) {
const value = env[key];
if (value !== '0' && value !== 'false') {
scrubbed.push(key);
}
delete env[key];
}There was a problem hiding this comment.
Good catch — you're absolutely right. is-in-ci checks key existence ('CI' in env), so '0' and 'false' still trigger CI detection since they're truthy strings in Node.js.
Fixed in 92bf17c: we now always delete env[key], but only push to the scrubbed warning list when the value is meaningful (not '0' or 'false'). Tests updated accordingly — all 7 pass.
`is-in-ci` checks key existence (`'CI' in env`), not the value. In Node.js, `'0'` and `'false'` are truthy strings, so leaving them in the environment still triggers CI detection and causes the interactive CLI to hang. Now we always delete CI/CONTINUOUS_INTEGRATION from the env, but only add to the scrubbed warning list when the value was meaningful (not '0' or 'false'), so callers can still suppress the warning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gemini-cli
bot
added
area/core
2 participants
Summary
Fixes #22452
The bundled build patches
is-in-civia an esbuild alias (seeesbuild.config.jsline 104) so thatinkstays in interactive mode regardless of CI env vars. However,npm run start(the dev server) bypasses esbuild and runs TypeScript directly, so the alias never applies.When
CIorCONTINUOUS_INTEGRATIONis set in the developer's shell — common for contributors working in CI-adjacent environments like GitLab runners, Jenkins, or any tool that setsCI=true—inkswitches to non-interactive mode and the CLI hangs indefinitely after displaying the ASCII art header, with no error message.Changes
scripts/scrub-ci-env.js— New utility that removesCIandCONTINUOUS_INTEGRATIONfrom an env object in-place, respecting the same falsy-value semantics ('0','false') thatis-in-civ2 uses.scripts/start.js— Import and callscrubCiEnv(env)before spawning the child process; print a warning to stderr when vars are scrubbed so developers know why their vars are missing.scripts/tests/scrub-ci-env.test.js— 7 unit tests covering truthy values, falsy values ('0','false'), empty string, both vars set, and no vars present.Why not just delete the env vars inline?
Extracting to a separate module makes the scrubbing logic independently testable and keeps
start.jsreadable. The utility mirrors the exact check thatis-in-civ2 performs (key in env && env[key] !== '0' && env[key] !== 'false'), so it stays correct even if the detection logic evolves.Test plan
scripts/tests/scrub-ci-env.test.js— all passnpx vitest run --config scripts/tests/vitest.config.ts) — 50/50 passnpm run test) — all passnpm run lint) — cleanCI=true npm run startnow shows the interactive prompt instead of hanging