Conversation
AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Gotcha: Git hooks require explicit loading of .env files to access API keys and other environment variables that are normally available in the main application context. AI-Confidence: high AI-Tags: git-hooks, environment-variables, dotenv, api-keys, configuration, execution-context AI-Lifecycle: project AI-Memory-Id: a4807174 AI-Source: llm-enrichment
AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Decision: The project uses dotenv for loading environment variables from .env files, specifically for hooks functionality. AI-Confidence: verified AI-Tags: dotenv, environment-variables, hooks, configuration, cli-tool, package-structure, binary AI-Lifecycle: project AI-Memory-Id: 5e159d36 AI-Source: llm-enrichment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Input
participant HookCmd as Hook Command
participant Git as Git CLI
participant Env as .env loader
participant Config as Hook Config
participant Hook as Hook Executor
Input->>HookCmd: invoke hook command (with cwd?)
HookCmd->>Git: git rev-parse --show-toplevel
Git-->>HookCmd: repo root (or fallback cwd)
HookCmd->>Env: load .env from repo root
Env-->>HookCmd: environment variables loaded
HookCmd->>Config: loadHookConfig(repoRoot)
Config-->>HookCmd: hook configuration
HookCmd->>Hook: isEventEnabled & process hooks
Hook-->>HookCmd: hook results
HookCmd-->>Input: exit / output
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds automatic .env loading to the git-mem hook command so hook executions can pick up API keys (notably ANTHROPIC_API_KEY) for LLM enrichment without requiring users to manually export environment variables.
Changes:
- Load
.env(fromcwd) at the start ofhookCommandexecution. - Add
dotenvas a runtime dependency. - Update
package-lock.jsonaccordingly.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/commands/hook.ts | Loads .env prior to reading hook config / dispatching events. |
| package.json | Adds dotenv dependency. |
| package-lock.json | Locks dotenv dependency resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Load .env from repository root for API keys (e.g., ANTHROPIC_API_KEY) | ||
| loadEnv({ path: join(cwd, '.env'), quiet: true }); | ||
|
|
There was a problem hiding this comment.
New behavior: hooks now load .env and mutate process.env before reading config/dispatching. There are existing unit/integration tests for hook behavior, but none assert that .env loading works (or that it doesn’t affect unrelated env vars). Adding an integration test that writes a .env in the temp repo and verifies the hook picks up a value from it would prevent regressions.
There was a problem hiding this comment.
Good point on test coverage. The existing hook integration tests cover the commit-msg flow but don't specifically test .env loading. Will defer adding that test to a follow-up since we've manually verified the end-to-end flow works.
| const cwd = input.cwd ?? process.cwd(); | ||
|
|
||
| // Load .env from repository root for API keys (e.g., ANTHROPIC_API_KEY) | ||
| loadEnv({ path: join(cwd, '.env'), quiet: true }); | ||
|
|
||
| const config = loadHookConfig(cwd); |
There was a problem hiding this comment.
The comment says this loads .env from the repository root, but the path is derived from cwd (stdin-provided working directory). If cwd is a subdirectory or not the repo top-level, this will silently miss the intended .env (and .git-mem.json), leading to hooks not picking up API keys. Consider resolving the git top-level (e.g., via git rev-parse --show-toplevel using cwd as the working dir) and loading .env from there, or adjust the comment/behavior accordingly.
There was a problem hiding this comment.
Fixed in 6abf8ea - now uses git rev-parse --show-toplevel to find the actual repo root, with graceful fallback to cwd if git fails.
AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Gotcha: use git root for .env loading, not cwd (GIT-94). AI-Agent: Claude-Code/2.1.42 AI-Confidence: medium AI-Tags: commands, typescript AI-Lifecycle: project AI-Memory-Id: 48348a38 AI-Source: heuristic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/hook.ts (1)
142-153: 🧹 Nitpick | 🔵 TrivialConsider loading .env only when event is enabled.
Currently,
.envis loaded before checking if the event is enabled (line 150). This means environment loading occurs even for disabled hooks. While the overhead is minimal, you could optimize by moving theisEventEnabledcheck earlier:const cwd = input.cwd ?? process.cwd(); const repoRoot = findGitRoot(cwd); + const config = loadHookConfig(repoRoot); + + if (!isEventEnabled(config.hooks, eventName)) { + clearTimeout(timer); + return; + } // Load .env from repository root for API keys (e.g., ANTHROPIC_API_KEY) loadEnv({ path: join(repoRoot, '.env'), quiet: true }); - - const config = loadHookConfig(repoRoot); - - if (!isEventEnabled(config.hooks, eventName)) { - clearTimeout(timer); - return; - }This avoids filesystem access for the
.envfile when the hook is disabled.
Summary
.envfrom repository rootANTHROPIC_API_KEYfor LLM enrichmentTest plan
.envfileAI-Source: llm-enrichmentshown in commit trailersCloses GIT-94
🤖 Generated with Claude Code
Summary by CodeRabbit
.envfile before processing, affecting hook behavior based on repo-level env values..env.