fix(cli): prevent GENIE_TUI_PANE env leak to child processes - #923
Conversation
Agents spawned from TUI panes inherit GENIE_TUI_PANE=left, causing genie.ts to render the TUI instead of running the command. Only enter TUI mode when there are zero CLI args AND the env var is set.
Only bare `genie` (no args) enters TUI mode. All subcommands (work, spawn, team, etc.) run normally. Clear the env var after the check so child processes never inherit it and accidentally enter TUI mode.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9621456978
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await launchTui(); | ||
| process.exit(0); | ||
| } | ||
| process.env.GENIE_TUI_PANE = undefined; |
There was a problem hiding this comment.
Assigning process.env.GENIE_TUI_PANE = undefined does not clear the variable in Node; it becomes the literal string 'undefined', so child processes still inherit GENIE_TUI_PANE and the leak this commit is trying to prevent remains. This can still alter behavior in any subprocess code that checks presence/truthiness of the variable rather than strict equality with 'left'.
Useful? React with 👍 / 👎.
![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 updates the TUI renderer activation logic in src/genie.ts to ensure it only triggers when no subcommands are provided and clears the GENIE_TUI_PANE environment variable to prevent inheritance by child processes. A review comment suggests simplifying the implementation by consolidating the environment variable cleanup and using a boolean flag to improve readability and reduce redundancy.
| if (process.env.GENIE_TUI_PANE === 'left' && args.length === 0) { | ||
| process.env.GENIE_TUI_PANE = undefined; | ||
| const { launchTui } = await import('./tui/index.js'); | ||
| await launchTui(); | ||
| process.exit(0); | ||
| } | ||
| process.env.GENIE_TUI_PANE = undefined; |
There was a problem hiding this comment.
While the logic is correct, unsetting process.env.GENIE_TUI_PANE in two different places can be simplified. You can determine if TUI renderer mode should be activated, unset the environment variable right away to prevent leaks, and then proceed. This makes the intent clearer and avoids redundancy. Using a dynamic import for the TUI module also helps manage module load-time dependencies and prevent potential circular cycles.
const isTuiRenderer = process.env.GENIE_TUI_PANE === 'left' && args.length === 0;
process.env.GENIE_TUI_PANE = undefined;
if (isTuiRenderer) {
const { launchTui } = await import('./tui/index.js');
await launchTui();
process.exit(0);
}References
- Use dynamic imports (require() or import()) to break circular dependency cycles that would otherwise occur at module load time.
1 participant
Agents spawned from TUI panes inherited GENIE_TUI_PANE=left, causing
genie work,genie spawnetc. to launch the TUI renderer insteadof running the command. Fix: only bare
genie(no args) enters TUImode, and clear the env var so children never inherit it.