Add repo setup instructions#731
Open
hanzjk wants to merge 3 commits into
Open
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds comprehensive test coverage for secret handling in agent creation and deployment, introduces a new local development setup guide, and updates documentation across the Agent Manager Service, Console, and Traces Observer Service to clarify configuration workflows and development procedures. A single environment variable was removed from the Agent Manager configuration template. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 4
🧹 Nitpick comments (2)
deployments/LOCAL-SETUP.md (1)
19-20: Nit: add a language hint to fenced code blocks.
markdownlintflags the ASCII-art architecture block (and a couple of others around Lines 14, 20, 112) as MD040. Tagging them astextsilences the lint without changing rendering.🧹 Proposed fix
-``` +```text ┌──────────────────────────────────────────────────────────────┐ │ Docker Compose (deployments/docker-compose.yml) │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments/LOCAL-SETUP.md` around lines 19 - 20, The ASCII-art fenced code blocks (e.g., the large architecture block starting with "┌─────────────────" and the other blocks flagged around lines 14, 20, 112) lack a language hint and trigger markdownlint MD040; update each such triple-backtick fence to include a language hint of "text" (i.e., replace ``` with ```text) so the blocks are treated as plain text and the linter warning is silenced while rendering stays identical.agent-manager-service/tests/create_agent_with_secrets_test.go (1)
260-278: Consider asserting each sensitive key individually.The current loop sets
foundSecretRef = trueif eitherDB_PASSWORDorAPI_SECRET_KEYis seen. If a regression causes one of the two sensitive entries to be dropped or emitted as plain value, the test still passes. Tracking both independently makes the guarantee stronger.🧪 Suggested tightening
- foundPlain := false - foundSecretRef := false + foundPlain := false + sensitiveSeen := map[string]bool{"DB_PASSWORD": false, "API_SECRET_KEY": false} for _, env := range createCall.Req.Configurations.Env { if env.Key == "DB_HOST" { foundPlain = true require.Equal(t, "localhost", env.Value) require.Nil(t, env.ValueFrom) } - if env.Key == "DB_PASSWORD" || env.Key == "API_SECRET_KEY" { - foundSecretRef = true + if _, ok := sensitiveSeen[env.Key]; ok { + sensitiveSeen[env.Key] = true require.NotNil(t, env.ValueFrom) require.NotNil(t, env.ValueFrom.SecretKeyRef) require.Equal(t, env.Key, env.ValueFrom.SecretKeyRef.Key) } } require.True(t, foundPlain, "Should have found plain env var DB_HOST") - require.True(t, foundSecretRef, "Should have found secret ref env vars") + for k, seen := range sensitiveSeen { + require.True(t, seen, "Expected sensitive env %s to be forwarded as secretKeyRef", k) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-manager-service/tests/create_agent_with_secrets_test.go` around lines 260 - 278, The test currently uses a single boolean foundSecretRef to track both DB_PASSWORD and API_SECRET_KEY which can mask missing entries; update the loop over createCall.Req.Configurations.Env to track and assert each sensitive key separately (e.g., use foundDBPassword and foundAPISecretKey) by checking env.Key == "DB_PASSWORD" and env.Key == "API_SECRET_KEY" independently, validating for each that env.ValueFrom and env.ValueFrom.SecretKeyRef are not nil and that env.ValueFrom.SecretKeyRef.Key equals env.Key, and then require.True for both flags instead of a single foundSecretRef; keep the existing DB_HOST plain-value assertions as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent-manager-service/README.md`:
- Around line 54-57: The README shows an incorrect cd path after cloning: the
repo is cloned as "agent-manager" but the docs use "cd
ai-agent-manager/agent-manager-service"; update the README to use the correct
path (e.g., replace the line "cd ai-agent-manager/agent-manager-service" with
"cd agent-manager/agent-manager-service") or alternatively update the git clone
command to clone into "ai-agent-manager" (e.g., "git clone <repo>
ai-agent-manager") so the subsequent cd works; edit the README.md entry
containing "git clone" and "cd ai-agent-manager/agent-manager-service"
accordingly.
- Around line 179-189: The README's manual migration command currently omits the
server flag so running `go run . -migrate` will start the HTTP server (main.go's
serverFlag defaults to true); update the docs to match the Make target by adding
`-server=false` to the example (e.g., `ENV_FILE_PATH=.env go run . -migrate
-server=false`) or, alternatively, change the default in main.go (serverFlag) to
false so invoking `-migrate` alone does not start the server—pick one approach
and update README and/or main.go accordingly.
In `@agent-manager-service/tests/deploy_agent_with_secrets_test.go`:
- Around line 162-200: The test currently doesn't exercise the 404 branch
because ComponentExistsFunc always returns true and DeployFunc returns a generic
error; update the mock so the 404 path is deterministic: modify
ComponentExistsFunc used in this test (in the subtest "Deploying agent that does
not exist should return 404") to return false when agentName ==
"nonexistent-agent-xyz" (or alternatively have DeployFunc return
utils.ErrNotFound when componentName == "nonexistent-agent-xyz"), then tighten
the assertion to require exactly http.StatusNotFound (rr.Code ==
http.StatusNotFound) so the test reliably verifies the 404 semantics; reference
the ComponentExistsFunc and DeployFunc mocks and the rr.Code assertion in the
subtest to locate the changes.
In `@traces-observer-service/README.MD`:
- Line 41: The README currently lists "Go: Version 1.20 or later" which
conflicts with the module requirement; update the README.MD entry to match
go.mod by changing the Go version requirement text from "1.20" to "1.25.0" (or
"1.25.0 or later") so it aligns with the service's go.mod Go directive and the
platform docs; locate the version string in README.MD and replace it
accordingly, verifying consistency with go.mod's "go 1.25" directive.
---
Nitpick comments:
In `@agent-manager-service/tests/create_agent_with_secrets_test.go`:
- Around line 260-278: The test currently uses a single boolean foundSecretRef
to track both DB_PASSWORD and API_SECRET_KEY which can mask missing entries;
update the loop over createCall.Req.Configurations.Env to track and assert each
sensitive key separately (e.g., use foundDBPassword and foundAPISecretKey) by
checking env.Key == "DB_PASSWORD" and env.Key == "API_SECRET_KEY" independently,
validating for each that env.ValueFrom and env.ValueFrom.SecretKeyRef are not
nil and that env.ValueFrom.SecretKeyRef.Key equals env.Key, and then
require.True for both flags instead of a single foundSecretRef; keep the
existing DB_HOST plain-value assertions as-is.
In `@deployments/LOCAL-SETUP.md`:
- Around line 19-20: The ASCII-art fenced code blocks (e.g., the large
architecture block starting with "┌─────────────────" and the other blocks
flagged around lines 14, 20, 112) lack a language hint and trigger markdownlint
MD040; update each such triple-backtick fence to include a language hint of
"text" (i.e., replace ``` with ```text) so the blocks are treated as plain text
and the linter warning is silenced while rendering stays identical.
🪄 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: CHILL
Plan: Pro
Run ID: 3e29f998-d04e-47fb-ae6c-4418940b42c8
📒 Files selected for processing (8)
agent-manager-service/.env.exampleagent-manager-service/README.mdagent-manager-service/tests/create_agent_with_secrets_test.goagent-manager-service/tests/deploy_agent_with_secrets_test.goagent-manager-service/tests/git_secret_test.goconsole/README.mddeployments/LOCAL-SETUP.mdtraces-observer-service/README.MD
💤 Files with no reviewable changes (1)
- agent-manager-service/.env.example
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.
Purpose
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Documentation
Tests