fix(auth): load agentmemory env for hooks and CLI - #535
Open
Lubrsy706 wants to merge 4 commits into
Open
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. |
Contributor
|
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 (27)
💤 Files with no reviewable changes (27)
📝 WalkthroughWalkthroughThis PR centralizes environment variable resolution across 25+ hook and script files by introducing a shared ChangesEnvironment Variable Resolution Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Hook as Hook/Script
participant AgentmemoryEnv as agentmemoryEnv(key)
participant ProcessEnv as process.env
participant EnvFile as ~/.agentmemory/.env
participant Cache as In-Memory Cache
Hook->>AgentmemoryEnv: get AGENTMEMORY_SECRET
AgentmemoryEnv->>ProcessEnv: check process.env[key]
alt key present in process.env
ProcessEnv-->>AgentmemoryEnv: return value
else key absent in process.env
AgentmemoryEnv->>Cache: check cached contents
alt cache empty (first call)
AgentmemoryEnv->>EnvFile: read & parse ~/.agentmemory/.env
EnvFile-->>Cache: populate cache
end
Cache-->>AgentmemoryEnv: return cached value or ""
end
AgentmemoryEnv-->>Hook: return secret (for auth header)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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)
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}
Contributor
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
Contributor
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
4 tasks
…rker-auth # Conflicts: # plugin/scripts/notification.mjs # plugin/scripts/post-tool-failure.mjs # plugin/scripts/post-tool-use.mjs # plugin/scripts/pre-compact.mjs # plugin/scripts/prompt-submit.mjs # plugin/scripts/session-start.mjs # plugin/scripts/subagent-start.mjs # plugin/scripts/subagent-stop.mjs # plugin/scripts/task-completed.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Release Notes
New Features
~/.agentmemory/.envin addition to environment variables, with process environment variables taking precedence. This enables persistent secret storage for the AgentMemory integration.Bug Fixes