feat(core): non-interactive env and PTY skip for Full Access shell exec#27157
feat(core): non-interactive env and PTY skip for Full Access shell exec#27157emersonbusson wants to merge 2 commits into
Conversation
Two related fixes for Full Access mode (--full-access / --approval-mode=
full_access) so shell commands don't hang on interactive sub-prompts.
1. Inject non-interactive env vars in shellExecutionService.ts so npm,
npx, apt, pip, yarn, and git auto-confirm instead of waiting at
"Ok to proceed? [y]". Pre-existing user values pass through via
`?? 'default'` coalesce. Vars injected when approvalMode === YOLO:
CI=1
npm_config_yes=true
npm_config_fund=false
npm_config_audit=false
YARN_ENABLE_INTERACTIVE=false
DEBIAN_FRONTEND=noninteractive
NEEDRESTART_MODE=a
GIT_TERMINAL_PROMPT=0
GH_PROMPT_DISABLED=1
GCM_INTERACTIVE=never
PIP_DISABLE_PIP_VERSION_CHECK=1
2. Skip the PTY in Full Access (shell.ts passes shouldUseNodePty=false
when approvalMode===YOLO). The child_process fallback uses
stdio:['ignore','pipe','pipe'], so sudo reads EOF on stdin and
exits with "a password is required" within ~1s instead of hanging
on the TTY password prompt.
Trade-off: TUI tools (vim, htop) no longer work inside Full Access.
That matches the intent — Full Access is fire-and-forget execution;
interactive TUI sessions don't belong in it.
Mirrors prior art: Codex CLI's UNIFIED_EXEC_ENV block at codex-rs/
core/src/unified_exec/process_manager.rs:60-71 (always-on env block)
and command.stdin(Stdio::null()) at codex-rs/utils/pty/src/pipe.rs:144
(stdin closure for fail-fast on sudo). Claude Code propagates a
similar env allow-list from the user's shell. This PR is the hybrid:
opinionated like Codex (auto-injects) but gated like a user opt-in
(YOLO only). Non-YOLO flows are unchanged.
Tests:
- shellExecutionService.test.ts: YOLO injects env / DEFAULT does not /
pre-existing user CI=0 and DEBIAN_FRONTEND=dialog preserved
- shell.test.ts: PTY enabled with interactive + non-YOLO / PTY
skipped in YOLO / PTY skipped when interactiveShell disabled
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 introduces optimizations for the Full Access (YOLO) execution mode to improve reliability and prevent process hangs. By forcing non-interactive behavior in common package managers and CLI tools, and bypassing PTY allocation for child processes, the system now handles fire-and-forget commands more predictably. These changes ensure that interactive prompts do not block execution, while maintaining existing behavior for standard, non-YOLO workflows. 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 the 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 counterproductive. 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
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Full Access' (YOLO) mode for shell execution, which automatically injects non-interactive environment variables (e.g., CI=1, DEBIAN_FRONTEND=noninteractive) to prevent package managers and installers from hanging on stdin prompts. It also modifies the shell tool to skip PTY allocation when in YOLO mode, ensuring that commands like sudo fail fast rather than waiting indefinitely for a password. Feedback was provided regarding the implementation of environment variable injection in shellExecutionService.ts, noting that the current use of nullish coalescing for defaults may be redundant due to existing logic that overwrites certain variables and violates the project's preference for defining defaults within the configuration schema.
| CI: baseEnv['CI'] ?? '1', | ||
| npm_config_yes: baseEnv['npm_config_yes'] ?? 'true', | ||
| npm_config_fund: baseEnv['npm_config_fund'] ?? 'false', | ||
| npm_config_audit: baseEnv['npm_config_audit'] ?? 'false', | ||
| YARN_ENABLE_INTERACTIVE: baseEnv['YARN_ENABLE_INTERACTIVE'] ?? 'false', | ||
| DEBIAN_FRONTEND: baseEnv['DEBIAN_FRONTEND'] ?? 'noninteractive', | ||
| NEEDRESTART_MODE: baseEnv['NEEDRESTART_MODE'] ?? 'a', | ||
| GIT_TERMINAL_PROMPT: baseEnv['GIT_TERMINAL_PROMPT'] ?? '0', | ||
| GH_PROMPT_DISABLED: baseEnv['GH_PROMPT_DISABLED'] ?? '1', | ||
| GCM_INTERACTIVE: baseEnv['GCM_INTERACTIVE'] ?? 'never', | ||
| PIP_DISABLE_PIP_VERSION_CHECK: | ||
| baseEnv['PIP_DISABLE_PIP_VERSION_CHECK'] ?? '1', |
There was a problem hiding this comment.
In the real application flow (defined in shell.ts), Full Access (YOLO) mode skips the PTY, which means shouldUseNodePty is false. This causes ShellExecutionService.execute to call childProcessFallback with isInteractive = false, which in turn triggers the if (!isInteractive) block at line 458. That block unconditionally overwrites GIT_TERMINAL_PROMPT, GH_PROMPT_DISABLED, and GCM_INTERACTIVE (lines 475-480), making the nullish coalescing in the YOLO block redundant and failing to respect user-provided values for these variables. To ensure user values are preserved as intended, the YOLO block should use sanitizedEnv as the source. Additionally, per repository rules, avoid using nullish coalescing for defaults in the code; these should be defined in the configuration schema to maintain a single source of truth.
| CI: baseEnv['CI'] ?? '1', | |
| npm_config_yes: baseEnv['npm_config_yes'] ?? 'true', | |
| npm_config_fund: baseEnv['npm_config_fund'] ?? 'false', | |
| npm_config_audit: baseEnv['npm_config_audit'] ?? 'false', | |
| YARN_ENABLE_INTERACTIVE: baseEnv['YARN_ENABLE_INTERACTIVE'] ?? 'false', | |
| DEBIAN_FRONTEND: baseEnv['DEBIAN_FRONTEND'] ?? 'noninteractive', | |
| NEEDRESTART_MODE: baseEnv['NEEDRESTART_MODE'] ?? 'a', | |
| GIT_TERMINAL_PROMPT: baseEnv['GIT_TERMINAL_PROMPT'] ?? '0', | |
| GH_PROMPT_DISABLED: baseEnv['GH_PROMPT_DISABLED'] ?? '1', | |
| GCM_INTERACTIVE: baseEnv['GCM_INTERACTIVE'] ?? 'never', | |
| PIP_DISABLE_PIP_VERSION_CHECK: | |
| baseEnv['PIP_DISABLE_PIP_VERSION_CHECK'] ?? '1', | |
| CI: sanitizedEnv['CI'], | |
| npm_config_yes: sanitizedEnv['npm_config_yes'], | |
| npm_config_fund: sanitizedEnv['npm_config_fund'], | |
| npm_config_audit: sanitizedEnv['npm_config_audit'], | |
| YARN_ENABLE_INTERACTIVE: sanitizedEnv['YARN_ENABLE_INTERACTIVE'], | |
| DEBIAN_FRONTEND: sanitizedEnv['DEBIAN_FRONTEND'], | |
| NEEDRESTART_MODE: sanitizedEnv['NEEDRESTART_MODE'], | |
| GIT_TERMINAL_PROMPT: sanitizedEnv['GIT_TERMINAL_PROMPT'], | |
| GH_PROMPT_DISABLED: sanitizedEnv['GH_PROMPT_DISABLED'], | |
| GCM_INTERACTIVE: sanitizedEnv['GCM_INTERACTIVE'], | |
| PIP_DISABLE_PIP_VERSION_CHECK: | |
| sanitizedEnv['PIP_DISABLE_PIP_VERSION_CHECK'], |
References
- Rely on the schema as the single source of truth for configuration defaults, avoiding redundant nullish coalescing operators.
- Use a sanitized environment for variable expansion to prevent extensions from bypassing environment variable redaction.
…lock Per @gemini-code-assist[bot]: in the real production flow, Full Access (YOLO) forces shouldUseNodePty=false (see shell.ts), so execute() runs the child_process path with isInteractive=false. The pre-existing `!isInteractive` block already sets GIT_TERMINAL_PROMPT, GH_PROMPT_DISABLED, and GCM_INTERACTIVE unconditionally — meaning the `??` coalesce in the YOLO block for those three was both redundant AND misleading (it would not actually preserve user values, since the earlier block overwrites them first). Remove the three duplicated entries from the YOLO block. The eight remaining vars (CI, npm_config_yes/fund/audit, YARN_ENABLE_INTERACTIVE, DEBIAN_FRONTEND, NEEDRESTART_MODE, PIP_DISABLE_PIP_VERSION_CHECK) are unique to this block; their `??` coalesce continues to respect pre-existing user values. Added inline comment documenting the composition with the `!isInteractive` block so future readers don't re-add the duplicates. Updated the YOLO test to assert the realistic composed environment (isInteractive=false, both blocks running) instead of the hypothetical "YOLO + interactive" case which shell.ts no longer produces.
|
Thanks @gemini-code-assist for the careful read. Pushed The remaining 8 vars ( Re: defining defaults in the schema instead of Test updated to reflect the realistic composed environment ( |
|
Thanks for the update and the clear explanation, @emersonbusson. The removal of the redundant environment variables is the right move, and the logic regarding the composition of the |
Summary
Two related fixes for Full Access mode (
--full-access/--approval-mode=full_access) so shell commands don't hang on interactive sub-prompts:npm,npx,apt,pip,yarn,gitauto-confirm instead of waiting atOk to proceed? [y].child_processpath (skip the PTY) sosudofails fast (a password is required) instead of waiting forever on a PTY password prompt.Both gated strictly on
approvalMode === YOLO— non-Full-Access flows are unchanged.Details
Env vars (
packages/core/src/services/shellExecutionService.ts) — extends the existing non-interactive env block (which only fires in!isInteractive) with a second branch firing onapprovalMode === YOLO, regardless of interactive UI. Pre-existing user values pass through via?? 'default'coalesce. Vars injected:PTY skip (
packages/core/src/tools/shell.ts) — passesshouldUseNodePty = getEnableInteractiveShell() && approvalMode !== YOLOtoShellExecutionService.execute. In Full Access the child_process fallback runs withstdio: ['ignore', 'pipe', 'pipe'], sosudoreads EOF on stdin and exits withsudo: a password is requiredwithin a second instead of hanging on the TTY password prompt.Trade-off accepted: TUI tools like
vim,htop, interactive REPLs no longer work inside Full Access. That matches the intent — Full Access is for fire-and-forget execution; interactive TUI sessions don't belong in it. The user can toggle out viaCtrl+Yif they need them.Prior art
codex-rs/core/src/unified_exec/process_manager.rs:60-71injects a fixed 10-var env block at every exec (NO_COLOR=1,TERM=dumb,LANG=C.UTF-8,PAGER=cat,CODEX_CI=1, ...). Always on. Codex also callscommand.stdin(Stdio::null())atcodex-rs/utils/pty/src/pipe.rs:144viaspawn_process_no_stdin_with_inherited_fds(), which is the same mechanism that makessudofail fast.CI,DEBIAN_FRONTEND,GIT_TERMINAL_PROMPT,NO_COLOR,TERM,COLORTERM,LANG,LC_ALL(from the user's environment to the child process). Also surfaces a detection message when a shell hangs: "The command is likely blocked on an interactive prompt. Kill this task and re-run with piped input (e.g.,echo y \| command) or a non-interactive flag if one exists."This PR is the hybrid: opinionated like Codex (auto-injects, doesn't just propagate), but gated like a user-opt-in feature (only on YOLO).
Related Issues
Closes #24707
Related to (variants of the same hang-on-stdin symptom):
npx op packup, the command never exits, and pressing Shift+Tab does not unfocus the input. #25782 —npx op packupnever exits; Shift+Tab can't recovernpm run start) — interactive mode hangs #22452 —CI_*env var scrub not applied in dev mode (interactive prompt loop)How to Validate
Run
gemini --full-accessin a fresh terminal, then:gemini --full-access -p "execute: env | sort | grep -E '^(CI|npm_config|DEBIAN_FRONTEND|NEEDRESTART|GIT_TERMINAL_PROMPT)='"→ output includesCI=1,npm_config_yes=true,DEBIAN_FRONTEND=noninteractive,NEEDRESTART_MODE=a,GIT_TERMINAL_PROMPT=0.gemini --full-access -p "execute: cd /tmp && rm -rf vite-x && npx create-vite@latest vite-x -- --template react"→ completes; previously hung atOk to proceed? [y].gemini --full-access -p "execute: sudo -n true"→ exits within ~1s withsudo: a password is required; previously hung indefinitely on the password prompt.CI=0already exported in the user's shell, run (1) again → output showsCI=0(the?? '1'coalesce respects pre-existing user values).--full-access(default mode): the env vars in (1) are NOT present; PTY is used as before.Pre-Merge Checklist
shellExecutionService.test.ts: YOLO injects env / DEFAULT does not / user-setCI=0andDEBIAN_FRONTEND=dialogpreservedshell.test.ts: PTY enabled when interactive + non-YOLO / PTY skipped in YOLO / PTY skipped when interactiveShell disabled regardless of mode