fix(auth): load agentmemory env for hooks and CLI#535
Conversation
|
@Lubrsy706 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a cached ~/.agentmemory/.env reader (agentmemoryEnv) and migrates plugin scripts, TypeScript hooks, and the CLI to use it for AGENTMEMORY_* configuration and centralized Authorization header construction; tests added to validate file-backed secret precedence and suppression. ChangesEnvironment Variable Resolution and Hook Configuration Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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
🤖 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/hooks/env.ts`:
- Around line 39-43: The agentmemoryEnv function currently treats empty-string
process.env values as absent and uses || when returning cachedEnv, so update
agentmemoryEnv to treat an explicitly set empty string as a valid override:
check for process.env[key] !== undefined (or use
Object.prototype.hasOwnProperty.call(process.env, key)) instead of a truthy
check to decide precedence, keep the existing cachedEnv lazy-init via
readAgentmemoryEnvFile(), and return cachedEnv[key] using the nullish coalescing
operator (??) so that an actual empty string from the cache is preserved rather
than coerced to "". Reference the agentmemoryEnv function and
cachedEnv/readAgentmemoryEnvFile usage when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6296165-d2f8-46a3-864a-03e20d79b24d
📒 Files selected for processing (30)
plugin/scripts/env-DODO3jxN.mjsplugin/scripts/notification.mjsplugin/scripts/post-commit.mjsplugin/scripts/post-tool-failure.mjsplugin/scripts/post-tool-use.mjsplugin/scripts/pre-compact.mjsplugin/scripts/pre-tool-use.mjsplugin/scripts/prompt-submit.mjsplugin/scripts/session-end.mjsplugin/scripts/session-start.mjsplugin/scripts/stop.mjsplugin/scripts/subagent-start.mjsplugin/scripts/subagent-stop.mjsplugin/scripts/task-completed.mjssrc/cli.tssrc/hooks/env.tssrc/hooks/notification.tssrc/hooks/post-commit.tssrc/hooks/post-tool-failure.tssrc/hooks/post-tool-use.tssrc/hooks/pre-compact.tssrc/hooks/pre-tool-use.tssrc/hooks/prompt-submit.tssrc/hooks/session-end.tssrc/hooks/session-start.tssrc/hooks/stop.tssrc/hooks/subagent-start.tssrc/hooks/subagent-stop.tssrc/hooks/task-completed.tstest/context-injection.test.ts
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 `@test/context-injection.test.ts`:
- Around line 224-246: The test currently treats authHeader === undefined as
proof no Authorization was sent but that could also mean the server never
received the request; modify the server handler in the createServer callback to
set a requestReceived flag (e.g., requestReceived = true) when invoked, keep
assigning authHeader there, and after runHook add an assertion
expect(requestReceived).toBe(true) (in addition to
expect(authHeader).toBeUndefined()) so the test only passes if the hook actually
hit the mock server; update references to authHeader, createServer, runHook and
expect accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1c1792d-4282-44d0-b49c-be52962e341d
📒 Files selected for processing (16)
plugin/scripts/env-B0rzso6b.mjsplugin/scripts/notification.mjsplugin/scripts/post-commit.mjsplugin/scripts/post-tool-failure.mjsplugin/scripts/post-tool-use.mjsplugin/scripts/pre-compact.mjsplugin/scripts/pre-tool-use.mjsplugin/scripts/prompt-submit.mjsplugin/scripts/session-end.mjsplugin/scripts/session-start.mjsplugin/scripts/stop.mjsplugin/scripts/subagent-start.mjsplugin/scripts/subagent-stop.mjsplugin/scripts/task-completed.mjssrc/hooks/env.tstest/context-injection.test.ts
1 participant
Summary
~/.agentmemory/.envplugin/scripts/*.mjshook outputsWhy
When
AGENTMEMORY_SECRETis set in~/.agentmemory/.env, the daemon enables API auth, but hook and CLI processes often do not inherit that shell variable. That makes hook calls to/session/start,/observe,/session/end,/consolidate-pipeline, etc. send no Bearer token even though the server requires one, which can silently break capture and downstream consolidation.This keeps explicit shell env overrides working while making the default
~/.agentmemory/.envconfiguration usable end-to-end.Addresses #518.
Validation
npm run buildnpm test -- test/context-injection.test.ts test/cli-connect.test.ts test/codex-plugin.test.ts test/stop-hook-recursion-guard.test.tsnpm testSummary by CodeRabbit
New Features
Tests