fix: Automatically load API keys from .env files at runtime #318
Conversation
WalkthroughAdds an env_loader module that discovers and loads .env files (cwd, ~/.cortex/.env, /etc/cortex/.env), exposes load_env via the public API, calls load_env() early in the CLI startup, adds python-dotenv as a dependency, and includes unit tests for discovery and loading behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Startup
participant Loader as cortex.env_loader
participant FS as File System
participant DotEnv as python-dotenv
participant ENV as os.environ
CLI->>Loader: load_env(override=False)
Loader->>Loader: get_env_file_locations()
Note over Loader: [cwd/.env, ~/.cortex/.env, /etc/cortex/.env]
Loader->>FS: stat/check each path
FS-->>Loader: existing .env paths
alt python-dotenv available
Loader->>DotEnv: load_dotenv(path, override=False)
DotEnv->>FS: read .env file
FS-->>DotEnv: file contents
DotEnv->>ENV: set variables (respecting override)
DotEnv-->>Loader: success
else python-dotenv missing
Loader-->>CLI: return [] (no-op)
end
Loader-->>CLI: list of loaded files
CLI->>CLI: continue init and API key checks (now read from ENV)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
requirements.txt (1)
9-22: Note: Duplicate PyYAML entries detected (pre-existing issue).PyYAML appears three times with inconsistent casing and versions:
- Line 9:
PyYAML>=6.0.0- Line 18:
pyyaml>=6.0.0- Line 22:
PyYAML==6.0.3While this is a pre-existing issue outside the scope of this PR, it could cause dependency resolution conflicts. Consider consolidating to a single entry in a future cleanup.
🧹 Nitpick comments (1)
cortex/env_loader.py (1)
142-147: Consider more robust key detection for accuracy.The simple substring match
if f"{key}=" in content:could produce false positives by matching:
- Commented lines:
# ANTHROPIC_API_KEY=value- Keys within strings or other contexts
Since this is a debugging utility, it's not critical, but consider using regex with line boundaries or leveraging python-dotenv's parsing for more accurate source attribution.
🔎 Alternative approach using regex
try: content = env_file.read_text() - if f"{key}=" in content: + import re + # Match KEY=VALUE at start of line, accounting for optional whitespace + if re.search(rf'^\s*{re.escape(key)}\s*=', content, re.MULTILINE): source = str(env_file) break except Exception: pass
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cortex/__init__.py(1 hunks)cortex/cli.py(1 hunks)cortex/env_loader.py(1 hunks)pyproject.toml(1 hunks)requirements.txt(1 hunks)tests/test_env_loader.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pycortex/env_loader.pycortex/__init__.pytests/test_env_loader.py
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
pyproject.tomlcortex/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_env_loader.py
🧬 Code graph analysis (3)
cortex/cli.py (1)
cortex/env_loader.py (1)
load_env(48-100)
cortex/__init__.py (1)
cortex/env_loader.py (1)
load_env(48-100)
tests/test_env_loader.py (1)
cortex/env_loader.py (4)
get_env_file_locations(22-45)load_env(48-100)find_env_files(103-113)get_api_key_sources(116-155)
🪛 GitHub Actions: CI
cortex/cli.py
[error] 1-1: Black formatting check failed. 5 files would be reformatted. Run 'black --write ' to fix formatting.
cortex/env_loader.py
[error] 1-1: Black formatting check failed. 5 files would be reformatted. Run 'black --write ' to fix formatting.
tests/test_env_loader.py
[error] 1-1: Black formatting check failed. 5 files would be reformatted. Run 'black --write ' to fix formatting.
🔇 Additional comments (12)
requirements.txt (1)
11-12: LGTM! Clear documentation of the new dependency.The addition of python-dotenv with a descriptive comment aligns well with the PR objectives to support .env file loading.
cortex/env_loader.py (4)
22-45: LGTM! Well-structured location discovery.The function correctly implements the prioritized search path specified in the PR objectives (current directory → user config → system config), with appropriate platform-specific handling for POSIX systems.
48-100: LGTM! Robust implementation with graceful error handling.The function correctly implements:
- Early application startup loading pattern
- Graceful degradation when python-dotenv is unavailable
- Respects existing environment variables by default (override=False)
- Per-file error handling that doesn't fail the entire operation
- Clear return value indicating which files were loaded
103-113: LGTM! Clean utility function.Simple and effective implementation for discovering existing .env files without loading them.
1-156: Run Black formatter to fix formatting issues.The CI pipeline reports that this file needs Black formatting. Run
black cortex/env_loader.pyto fix automatically.⛔ Skipped due to learnings
Learnt from: CR Repo: cortexlinux/cortex PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-11T12:03:24.071Z Learning: Applies to **/*.py : Follow PEP 8 style guidecortex/cli.py (1)
716-719: LGTM! Proper placement and clear intent.The environment loading is correctly positioned at the very start of
main(), before any parser initialization or API key checks. The local import and explanatory comments make the intent clear.cortex/__init__.py (1)
2-7: LGTM! Proper public API exposure.The new
load_envfunction is correctly imported and exported in__all__, making it available as part of the package's public API.tests/test_env_loader.py (4)
17-22: Clever approach to avoid heavy dependencies in tests.The dynamic module loading technique avoids triggering cortex package initialization and its dependencies (rich, etc.), which is a smart optimization for test isolation.
25-63: LGTM! Comprehensive tests for location discovery.The tests thoroughly verify:
- Return type correctness
- Inclusion of all expected locations
- Priority ordering (cwd first)
- Platform-specific behavior (POSIX check)
65-174: LGTM! Excellent coverage of load_env behavior.The tests verify all critical scenarios:
- Empty directory (no files)
- Loading from cwd
- Default behavior (no override)
- Override mode
- Graceful handling of missing python-dotenv
Proper use of temp directories, mocking, and environment restoration in finally blocks ensures test isolation.
243-330: LGTM! Strong integration test coverage.The integration tests verify end-to-end functionality:
- Individual key loading (Anthropic, OpenAI)
- Multiple keys from a single .env file
- Actual environment population
This validates the complete workflow described in the PR objectives.
pyproject.toml (1)
49-49: python-dotenv dependency is valid and secure.python-dotenv 1.0.0 has no known vulnerabilities, and the latest version is 1.2.1 with no known vulnerabilities. The version constraint
>=1.0.0is appropriate.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_env_loader.py (1)
166-173: Suggest renaming or enhancing the ImportError test.The test name
test_handles_missing_dotenv_gracefullyimplies it verifiesImportErrorhandling, but it only confirms the function doesn't crash when dotenv is available. Consider either:
- Renaming to
test_load_env_works_with_dotenv_installed, or- Using
mock.patchto simulateImportErrorand verify the function returns an empty list.🔎 Example: Properly test ImportError handling
- def test_handles_missing_dotenv_gracefully(self): - """Should handle ImportError for python-dotenv gracefully.""" - # The function should gracefully return empty list when dotenv unavailable - # Since dotenv IS installed in test env, we just verify the function works - result = env_loader.load_env() - - # Should not raise, should return list (possibly empty or with loaded files) - assert isinstance(result, list) + def test_handles_missing_dotenv_gracefully(self): + """Should handle ImportError for python-dotenv gracefully.""" + # Mock the import to raise ImportError + with mock.patch('builtins.__import__', side_effect=ImportError): + result = env_loader.load_env() + + # Should return empty list when dotenv unavailable + assert result == []
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cortex/cli.py(1 hunks)cortex/env_loader.py(1 hunks)src/intent/clarifier.py(1 hunks)src/intent/llm_agent.py(4 hunks)tests/test_env_loader.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/intent/llm_agent.py
- src/intent/clarifier.py
🚧 Files skipped from review as they are similar to previous changes (1)
- cortex/env_loader.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/cli.pytests/test_env_loader.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_env_loader.py
🧬 Code graph analysis (2)
cortex/cli.py (1)
cortex/env_loader.py (1)
load_env(48-100)
tests/test_env_loader.py (1)
cortex/env_loader.py (4)
get_env_file_locations(22-45)load_env(48-100)find_env_files(103-113)get_api_key_sources(116-155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Package
🔇 Additional comments (7)
cortex/cli.py (1)
716-720: LGTM! Environment loading correctly placed at startup.The placement of
load_env()at the very beginning ofmain()is optimal—it ensures API keys from.envfiles are available before any code accessesos.environ. Using default parameters (no override, silent mode) is appropriate for CLI usage.tests/test_env_loader.py (6)
17-22: Good approach to avoid dependency issues during testing.Using
importlibto load the module directly bypasses potential side effects fromcortex/__init__.py. While this means tests don't validate the actual import path, it's a reasonable trade-off for test reliability.
25-63: Comprehensive test coverage for env file location discovery.The tests properly validate the priority order, platform-specific behavior (POSIX), and expected paths. Good use of
pytest.mark.skipiffor platform-specific tests.
68-174: Excellent test isolation with proper state restoration.All tests correctly save and restore
os.environandos.getcwd()using try/finally blocks. The cleanup pattern ensures tests don't leak state even if assertions fail.
176-209: LGTM! Clean tests for env file discovery.Tests properly verify both the empty case and the discovery of existing files, with appropriate isolation using temporary directories and mocking.
211-241: Good coverage for API key source detection.Tests verify the function returns the expected dictionary structure with all relevant API keys and correctly handles missing keys.
243-329: Comprehensive integration tests validate end-to-end behavior.The integration tests properly verify that API keys are loaded from
.envfiles intoos.environfor single-key and multi-key scenarios. Consistent test structure and proper cleanup.
|
@mikejmorgan-ai Please review this PR. |
2 participants
Summary
Fixes the bug where API keys stored in
.envfiles were not reliably loaded by Cortex at runtime.Problem
Users had to manually
export OPENAI_API_KEY=...for Cortex to detect API keys, even when a valid.envfile existed in the project directory.Solution
python-dotenvas a dependencycortex/env_loader.pymodule that loads.envfiles from multiple locations:./.env)~/.cortex/.env)/etc/cortex/.env)load_env()at the start ofmain()before any API key accessTesting
tests/test_env_loader.pyChanges
requirements.txt- Added python-dotenvpyproject.toml- Added python-dotenv to dependenciescortex/env_loader.py- New module (env loading logic)cortex/cli.py- Call load_env() at startupcortex/__init__.py- Export load_env functiontests/test_env_loader.py- New test fileClosed: #317
Summary by CodeRabbit
New Features
Tests
Refactor / Style
✏️ Tip: You can customize this high-level summary in your review settings.