feat(config): add explicit provider env-file loading#1668
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (12)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{src/commands/**/*.ts,src/entrypoints/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughAdds explicit Changes--provider-env-file feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/envFile.ts`:
- Around line 125-137: The issue is that the closingQuoteIdx calculation using
value.indexOf(quote, 1) doesn't account for escaped quotes, so it incorrectly
stops at the first escaped quote (like the \" in JSON strings) rather than the
actual closing quote. Modify the logic that finds the closingQuoteIdx to
properly iterate through the string and skip over escaped quotes (quotes
preceded by a backslash), continuing until it finds an unescaped quote character
that matches the opening quote. This will allow valid entries with escaped
quotes like JSON objects to be parsed correctly without triggering the
"unexpected content after quoted value" error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b785a40-9c82-462f-8260-cbb0a05e156a
📒 Files selected for processing (9)
.env.exampleREADME.mddocs/non-technical-setup.mddocs/quick-start-mac-linux.mddocs/quick-start-windows.mdsrc/entrypoints/cli.tsxsrc/main.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (6)
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
docs/quick-start-mac-linux.mddocs/quick-start-windows.mddocs/non-technical-setup.mdREADME.mdsrc/entrypoints/cli.tsxsrc/main.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/quick-start-mac-linux.mddocs/quick-start-windows.mddocs/non-technical-setup.mdREADME.md
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
docs/quick-start-mac-linux.mddocs/quick-start-windows.mddocs/non-technical-setup.mdREADME.mdsrc/entrypoints/cli.tsxsrc/main.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/entrypoints/cli.tsxsrc/main.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/entrypoints/cli.tsxsrc/main.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/envFile.test.ts
🪛 ast-grep (0.43.0)
src/utils/envFile.test.ts
[warning] 187-187: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(Failed to load --provider-env-file at ${filePath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}:)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🔇 Additional comments (3)
src/main.tsx (1)
957-960: LGTM!src/entrypoints/cli.tsx (1)
81-99: LGTM!src/utils/envFile.test.ts (1)
1-241: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Preserve explicit env-file values across saved settings
src/entrypoints/cli.tsx:83
The new loader applies--provider-env-filebeforeapplySafeConfigEnvironmentVariables()and saved profile startup run, but those later steps can stillObject.assignglobal/user/policy/settings env back overprocess.env. That means a user who already has a saved/providerprofile orsettings.envcan runopenclaude --provider-env-file .envand still get the old saved provider key/model/base URL instead of the file they explicitly passed.--provideralready has a remembered/reapplied path to survive the same settings merge; please give--provider-env-fileequivalent explicit-CLI precedence, or move its loading to the point where later saved settings/profile hydration cannot clobber it.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/entrypoints/cli.test.ts`:
- Around line 89-109: The test in cli.test.ts currently validates only source
code ordering by searching for function names like Object.assign,
applySafeConfigEnvironmentVariables, reapplyProviderEnvFileValues, and
applyStartupEnvFromProfile using indexOf checks. This tests implementation
details rather than actual runtime behavior. Replace this test with a
behavior-level regression test that exercises the startup merge path by setting
up provider env file values, applying settings environment variables, applying
startup profile environment, and then asserting that the final environment
values still contain the provider env file values with correct precedence after
all merges complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 41f3df20-1f0a-48fa-b911-5cd5dcb0dcea
📒 Files selected for processing (4)
src/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise in code
Files:
src/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/entrypoints/cli.test.tssrc/entrypoints/cli.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/entrypoints/cli.test.tssrc/utils/envFile.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/utils/envFile.test.tssrc/utils/envFile.ts
🔇 Additional comments (3)
src/utils/envFile.ts (1)
276-277: LGTM!Also applies to: 279-297
src/entrypoints/cli.tsx (1)
72-75: LGTM!Also applies to: 96-96, 130-130, 141-141
src/utils/envFile.test.ts (1)
14-14: LGTM!Also applies to: 159-159, 163-181
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found a couple of issues that still need to be addressed.
Findings
-
[P2] Preserve env-file values through the full settings apply path
src/utils/managedEnv.ts:198
My earlier review request is only partially fixed in the current patch.cli.tsxnow reapplies the env-file values after the early safe settings/profile merges, but real startup later callsapplyConfigEnvironmentVariables()in both interactive and--printpaths, and that function stillObject.assigns the merged settings env back intoprocess.envwhile only restoring--providerviareapplyRememberedProviderFlag(). A user can still runopenclaude --provider-env-file .env -p ...from a trusted project with project/usersettings.envvalues and have those later settings replace the file key/model/base URL before the request runs. Please carry the remembered env-file values through the same full settings application path, or otherwise ensure later settings/profile hydration cannot clobber the explicitly loaded file. -
[P2] Allow the documented Azure API version env var
src/utils/envFile.ts:22
The new loader rejects every key not inALLOWED_ENV_FILE_KEYS, but the documented Azure resource-base setup requiresAZURE_OPENAI_API_VERSION, and the OpenAI shim reads that variable when building the Azure chat-completions URL. With this PR, putting the documented Azure env block into a file and runningopenclaude --provider-env-file .envfails withUnsupported variable AZURE_OPENAI_API_VERSION, so the new file-based workflow does not work for one of the supported provider configurations. Please include the Azure API-version variable in the provider/setup allowlist or adjust the docs/runtime contract accordingly.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one issue that still needs to be addressed.
Findings
- [P2] Include the documented setup variables in the env-file allowlist
src/utils/envFile.ts:5
The new loader rejects any key missing fromALLOWED_ENV_FILE_KEYS, but it still omits documented setup variables that users reasonably put in the same.envworkflow this PR introduces. For example,.env.exampleand the web-search provider docs documentWEB_SEARCH_API,WEB_URL_TEMPLATE,WEB_CUSTOM_ALLOW_PRIVATE, and related custom web-search variables, whiledocs/advanced-setup.mddocumentsCODEX_AUTH_JSON_PATHfor Codex auth-file setup. Runningopenclaude --provider-env-file .envwith either documented block now exits before startup withUnsupported variable WEB_SEARCH_APIorUnsupported variable CODEX_AUTH_JSON_PATH. Please either allow the documented provider/setup variables that are safe for this explicit loader, or update the docs so users are not told this file-based setup path supports configurations that it rejects.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
|
please rebase to main and fix conflicts @chioarub |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/entrypoints/cli.test.ts (1)
226-255:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a regression for
--providerwinning over env-file values.This test covers settings/profile overwrites, but not the conflicting explicit CLI provider case. Add a case that loads an env file for one provider, applies/remembers
--providerfor another provider, runs both reapply checkpoints, and asserts the provider flag remains authoritative.As per coding guidelines, “Add or update tests when the change affects behavior” and “Test the exact provider/model path you changed when possible.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entrypoints/cli.test.ts` around lines 226 - 255, Add a new test case that validates the precedence of explicit --provider CLI arguments over env-file values in conflict scenarios. The test should load an env file containing one provider configuration using loadEnvFile, then apply a different provider via a --provider CLI argument (simulating the CLI flag), call applyLoadedEnvFileValues twice to simulate the reapply checkpoints, and finally assert that the environment variables reflect the CLI provider values rather than the env-file values, demonstrating that the explicit provider flag remains authoritative throughout the merging process.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 110-134: The "Background sessions" documentation section in the
README appears to be unrelated to the PR's stated scope of adding the
--provider-env-file CLI option. Either update the PR description to clarify that
background session documentation is intentionally included in the expanded
scope, or move this entire section (the Background sessions heading and all
associated content showing openclaude --bg, ps, logs, kill, and attach commands)
to a separate pull request focused specifically on background sessions feature
documentation.
In `@src/entrypoints/cli.tsx`:
- Line 143: The reapplyProviderEnvFileValues function on line 143 needs to
ensure the --provider flag takes precedence over env-file values. The issue is
that reapplyRememberedEnvFileValues() uses Object.assign to restore env-file
settings, which can overwrite the --provider flag if called after the provider
is set. Modify the reapplyProviderEnvFileValues implementation to reapply the
remembered provider flag value after restoring any env-file values, ensuring
that standalone calls to reapplyRememberedEnvFileValues on lines 231 and 240 do
not override the provider precedence. This ensures that when both --provider and
env-file contain conflicting settings, the --provider flag maintains its
intended precedence.
In `@src/main.tsx`:
- Line 2522: The interactive resume path's hooksPromise guard at line 2353 does
not suppress the startup hooks when fromPr is set, while the headless path at
line 2522 correctly does so. Add `|| options.fromPr` to the condition that
determines whether to call `processSessionStartHooks('startup')` in the
interactive path to mirror the suppression logic from the headless path. This
ensures startup hooks are consistently suppressed for PR-resume flows in both
code paths, preventing duplicate or conflicting resume lifecycle output.
---
Outside diff comments:
In `@src/entrypoints/cli.test.ts`:
- Around line 226-255: Add a new test case that validates the precedence of
explicit --provider CLI arguments over env-file values in conflict scenarios.
The test should load an env file containing one provider configuration using
loadEnvFile, then apply a different provider via a --provider CLI argument
(simulating the CLI flag), call applyLoadedEnvFileValues twice to simulate the
reapply checkpoints, and finally assert that the environment variables reflect
the CLI provider values rather than the env-file values, demonstrating that the
explicit provider flag remains authoritative throughout the merging process.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a90cc4c9-bce2-489b-9cff-e8ffb891f950
📒 Files selected for processing (4)
README.mdsrc/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/main.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
README.mdsrc/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
README.mdsrc/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.md
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
README.mdsrc/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsxsrc/entrypoints/cli.tsxsrc/entrypoints/cli.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/entrypoints/cli.test.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/entrypoints/cli.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/entrypoints/cli.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/entrypoints/cli.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/entrypoints/cli.test.ts
🔇 Additional comments (4)
README.md (1)
108-108: Documentation note for --provider-env-file is accurate and well-positioned.The new note correctly documents the non-auto-loading behavior, recommends the secure
/providerpath first, and properly guides users toward environment-based setup with allowlist semantics. The guidance to export runtime/debug variables separately aligns with the implementation's rejection of process-control variables.src/main.tsx (1)
958-960: LGTM!Also applies to: 2749-2749, 3751-3758
src/entrypoints/cli.tsx (1)
70-142: LGTM!Also applies to: 153-203, 221-230, 246-300, 310-315, 320-320, 535-541, 547-550
src/entrypoints/cli.test.ts (1)
6-23: LGTM!Also applies to: 25-96, 257-287, 289-449
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/entrypoints/cli.test.ts (1)
226-255:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a regression for
--providerwinning over env-file values.This test covers settings/profile overwrites, but not the conflicting explicit CLI provider case. Add a case that loads an env file for one provider, applies/remembers
--providerfor another provider, runs both reapply checkpoints, and asserts the provider flag remains authoritative.As per coding guidelines, “Add or update tests when the change affects behavior” and “Test the exact provider/model path you changed when possible.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entrypoints/cli.test.ts` around lines 226 - 255, Add a new test case that validates the precedence of explicit --provider CLI arguments over env-file values in conflict scenarios. The test should load an env file containing one provider configuration using loadEnvFile, then apply a different provider via a --provider CLI argument (simulating the CLI flag), call applyLoadedEnvFileValues twice to simulate the reapply checkpoints, and finally assert that the environment variables reflect the CLI provider values rather than the env-file values, demonstrating that the explicit provider flag remains authoritative throughout the merging process.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 110-134: The "Background sessions" documentation section in the
README appears to be unrelated to the PR's stated scope of adding the
--provider-env-file CLI option. Either update the PR description to clarify that
background session documentation is intentionally included in the expanded
scope, or move this entire section (the Background sessions heading and all
associated content showing openclaude --bg, ps, logs, kill, and attach commands)
to a separate pull request focused specifically on background sessions feature
documentation.
In `@src/entrypoints/cli.tsx`:
- Line 143: The reapplyProviderEnvFileValues function on line 143 needs to
ensure the --provider flag takes precedence over env-file values. The issue is
that reapplyRememberedEnvFileValues() uses Object.assign to restore env-file
settings, which can overwrite the --provider flag if called after the provider
is set. Modify the reapplyProviderEnvFileValues implementation to reapply the
remembered provider flag value after restoring any env-file values, ensuring
that standalone calls to reapplyRememberedEnvFileValues on lines 231 and 240 do
not override the provider precedence. This ensures that when both --provider and
env-file contain conflicting settings, the --provider flag maintains its
intended precedence.
In `@src/main.tsx`:
- Line 2522: The interactive resume path's hooksPromise guard at line 2353 does
not suppress the startup hooks when fromPr is set, while the headless path at
line 2522 correctly does so. Add `|| options.fromPr` to the condition that
determines whether to call `processSessionStartHooks('startup')` in the
interactive path to mirror the suppression logic from the headless path. This
ensures startup hooks are consistently suppressed for PR-resume flows in both
code paths, preventing duplicate or conflicting resume lifecycle output.
---
Outside diff comments:
In `@src/entrypoints/cli.test.ts`:
- Around line 226-255: Add a new test case that validates the precedence of
explicit --provider CLI arguments over env-file values in conflict scenarios.
The test should load an env file containing one provider configuration using
loadEnvFile, then apply a different provider via a --provider CLI argument
(simulating the CLI flag), call applyLoadedEnvFileValues twice to simulate the
reapply checkpoints, and finally assert that the environment variables reflect
the CLI provider values rather than the env-file values, demonstrating that the
explicit provider flag remains authoritative throughout the merging process.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a90cc4c9-bce2-489b-9cff-e8ffb891f950
📒 Files selected for processing (4)
README.mdsrc/entrypoints/cli.test.tssrc/entrypoints/cli.tsxsrc/main.tsx
📜 Review details
🔇 Additional comments (4)
README.md (1)
108-108: Documentation note for --provider-env-file is accurate and well-positioned.The new note correctly documents the non-auto-loading behavior, recommends the secure
/providerpath first, and properly guides users toward environment-based setup with allowlist semantics. The guidance to export runtime/debug variables separately aligns with the implementation's rejection of process-control variables.src/main.tsx (1)
958-960: LGTM!Also applies to: 2749-2749, 3751-3758
src/entrypoints/cli.tsx (1)
70-142: LGTM!Also applies to: 153-203, 221-230, 246-300, 310-315, 320-320, 535-541, 547-550
src/entrypoints/cli.test.ts (1)
6-23: LGTM!Also applies to: 25-96, 257-287, 289-449
🛑 Comments failed to post (3)
README.md (1)
110-134:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope concern: "Background sessions" documentation appears unrelated to --provider-env-file feature.
Lines 110–134 document
openclaude --bg,ps,logs,kill, andattachcommands for background session management. Per the PR objectives and review stack context, this PR is scoped to adding the--provider-env-fileCLI option and related docs updates. The background sessions feature is not mentioned in the PR summary, and appears to be bundled as an unrelated change.Per CONTRIBUTING.md, bundling unrelated features without prior discussion may trigger closure without review. If background sessions are intentional, update the PR description to clarify the expanded scope; if not, consider moving this documentation to a separate PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 110 - 134, The "Background sessions" documentation section in the README appears to be unrelated to the PR's stated scope of adding the --provider-env-file CLI option. Either update the PR description to clarify that background session documentation is intentionally included in the expanded scope, or move this entire section (the Background sessions heading and all associated content showing openclaude --bg, ps, logs, kill, and attach commands) to a separate pull request focused specifically on background sessions feature documentation.src/entrypoints/cli.tsx (1)
143-143:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
--providerprecedence after env-file reapply.
reapplyRememberedEnvFileValues()overwrites viaObject.assign, and the standalone calls on Lines 231 and 240 run after the managed-env path that reapplies the remembered--providerflag last. With both flags, an env file containingCLAUDE_CODE_USE_OPENAI=1can be restored after--provider gemini, so validation or background spawn can see the wrong provider. Reapply the remembered provider flag after these restores, or share one env-file-then-provider helper.Proposed fix
- let reapplyProviderEnvFileValues = () => {} + let reapplyProviderEnvFileValues = () => {} + let reapplyProviderFlagValues = () => {} + const reapplyExplicitProviderInputs = () => { + reapplyProviderEnvFileValues() + reapplyProviderFlagValues() + } @@ - const { applyProviderFlagFromArgs } = await importers.providerFlag(); + const { + applyProviderFlagFromArgs, + reapplyRememberedProviderFlag, + } = await importers.providerFlag() + reapplyProviderFlagValues = reapplyRememberedProviderFlag @@ - reapplyProviderEnvFileValues() + reapplyExplicitProviderInputs() @@ - reapplyProviderEnvFileValues() + reapplyExplicitProviderInputs()Also applies to: 208-208, 231-240
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entrypoints/cli.tsx` at line 143, The reapplyProviderEnvFileValues function on line 143 needs to ensure the --provider flag takes precedence over env-file values. The issue is that reapplyRememberedEnvFileValues() uses Object.assign to restore env-file settings, which can overwrite the --provider flag if called after the provider is set. Modify the reapplyProviderEnvFileValues implementation to reapply the remembered provider flag value after restoring any env-file values, ensuring that standalone calls to reapplyRememberedEnvFileValues on lines 231 and 240 do not override the provider precedence. This ensures that when both --provider and env-file contain conflicting settings, the --provider flag maintains its intended precedence.src/main.tsx (1)
2522-2522:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror
fromPrhook suppression in the interactive resume path.Line 2522 handles the headless path, but the interactive
hooksPromiseguard at Line 2353 still startsprocessSessionStartHooks('startup')before theoptions.fromPrresume branch at Line 3263. That can fire startup hooks for a PR-resume flow and duplicate or clobber resume lifecycle output.Proposed fix
- const hooksPromise = initOnly || init || maintenance || isNonInteractiveSession || options.continue || options.resume ? null : processSessionStartHooks('startup', { + const hooksPromise = initOnly || init || maintenance || isNonInteractiveSession || options.continue || options.resume || options.fromPr ? null : processSessionStartHooks('startup', { agentType: mainThreadAgentDefinition?.agentType, model: resolvedInitialModel });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.tsx` at line 2522, The interactive resume path's hooksPromise guard at line 2353 does not suppress the startup hooks when fromPr is set, while the headless path at line 2522 correctly does so. Add `|| options.fromPr` to the condition that determines whether to call `processSessionStartHooks('startup')` in the interactive path to mirror the suppression logic from the headless path. This ensures startup hooks are consistently suppressed for PR-resume flows in both code paths, preventing duplicate or conflicting resume lifecycle output.
3 participants
Summary
Adds an explicit
--provider-env-file <path>option for users who prefer file-based provider setup while keeping project.envfiles opt-in.Problem
.env.exampleencouraged copying values into.env, but OpenClaude did not automatically load that file. Users could follow the template and still see missing provider key errors.Chosen solution
.envsyntax:KEY=VALUE, quoted values, comments, andexport KEY=VALUE..envis not auto-loaded and that/providerremains the recommended guided setup path.Security model
.envfiles.PATH,NODE_OPTIONS, andLD_PRELOADare rejected.Validation
bun test --max-concurrency=1— 4093 pass, 0 fail, 10100 expectations.bun run typecheck— passed.bun run build— passed; CLI and SDK bundles built and validated.node bin/openclaude --version—0.18.0 (OpenClaude)..envloading, unsupported variable rejection, and lowercase env-name rejection.Summary by CodeRabbit
--provider-env-file <path>option to explicitly load provider-scoped.envvariables from one or more files during startup./fromPr, plus clearer validation and fail-fast errors for malformed/unsupported env lines..env.example, README, and Quick Start guides with optional.envusage, ordering notes, and troubleshooting.