feat: add CodeRabbit integration for AI-powered code review#1145
feat: add CodeRabbit integration for AI-powered code review#1145jeremyeder wants to merge 3 commits intoambient-code:mainfrom
Conversation
Add CodeRabbit as a native integration in ACP, enabling AI-powered code review inside agentic sessions. Backend: - CodeRabbit auth handlers: connect, disconnect, status, test endpoints - Per-user K8s Secret credential storage (coderabbit-credentials) - API key validation against CodeRabbit health endpoint - Runtime credential fetch for sessions with RBAC - CodeRabbit included in unified integrations status Frontend: - CodeRabbit integration card on Integrations page - API client, React Query hooks, Next.js proxy routes - CodeRabbit shown in session settings integrations panel Runner: - Fetch CodeRabbit API key at session startup - Set CODERABBIT_API_KEY env var for CLI authentication Config: - .coderabbit.yaml with project-specific review rules and custom checks - Pre-commit hook with rate limit handling and 5-minute timeout - .worktrees/ added to .gitignore Tests: - 7 Ginkgo tests for CodeRabbit auth handlers - Frontend integrations panel test updated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jeremyeder
force-pushed
the
feat/coderabbit-integration
branch
from
736e686 to
f995418
Compare
📝 WalkthroughWalkthroughThis pull request adds a CodeRabbit integration: backend handlers and Kubernetes Secret storage for credentials, frontend UI/API and React Query hooks to connect/status/disconnect/test, agentic-runner runtime credential injection, a pre-commit CodeRabbit review hook, and supporting tests and docs. Changes
Sequence DiagramssequenceDiagram
participant User as User (Browser)
participant FrontendAPI as Frontend API
participant Backend as Backend Service
participant K8s as Kubernetes
participant CodeRabbit as CodeRabbit API
participant Validator as Validator
User->>FrontendAPI: POST /api/auth/coderabbit/connect {apiKey}
FrontendAPI->>FrontendAPI: buildForwardHeadersAsync()
FrontendAPI->>Backend: POST /auth/coderabbit/connect {apiKey}
Backend->>Validator: ValidateCodeRabbitAPIKey(apiKey)
Validator->>CodeRabbit: GET /api/v1/health (Bearer apiKey)
CodeRabbit-->>Validator: 200 OK / error
Validator-->>Backend: validation result
alt Validation success
Backend->>K8s: Get `coderabbit-credentials` Secret
K8s-->>Backend: Secret (or NotFound)
Backend->>K8s: Create/Update Secret with user JSON data (retry on conflict)
K8s-->>Backend: updated Secret
Backend-->>FrontendAPI: 200 {message: "Connected"}
FrontendAPI-->>User: 200 OK
else Validation failure
Backend-->>FrontendAPI: 400 {error: "..."}
FrontendAPI-->>User: error response
end
sequenceDiagram
participant Session as Agentic Session
participant Runner as Ambient Runner
participant Backend as Backend Service
participant K8s as Kubernetes
Runner->>Backend: GET /api/projects/:proj/.../credentials/coderabbit
Backend->>K8s: Read `coderabbit-credentials` Secret
K8s-->>Backend: Secret data
alt Credentials found
Backend-->>Runner: 200 {apiKey: "..."}
Runner->>Runner: set ENV CODERABBIT_API_KEY
Runner-->>Session: continue with credentials
else Credentials missing
Backend-->>Runner: 404
Runner->>Runner: log warning, skip CodeRabbit
Runner-->>Session: continue without credentials
end
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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: 4
🧹 Nitpick comments (1)
.gitignore (1)
143-143: Remove duplicate.worktrees/ignore entry.Line 143 duplicates the existing rule at Line 94. Keeping a single entry avoids drift/confusion over time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 143, Remove the duplicate .worktrees/ entry from .gitignore: locate the repeated ignore rule ".worktrees/" and delete the later occurrence so only one .worktrees/ line remains in the file to avoid redundant entries and confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/backend/handlers/coderabbit_auth.go`:
- Around line 153-173: The Secret create/read/update calls are using the
package-global K8sClient (e.g., K8sClient.CoreV1().Secrets(...)) which bypasses
per-request RBAC; update the handlers in coderabbit_auth.go to use the
request-scoped client returned by GetK8sClientsForRequest(c) (e.g., reqK8s) for
all Secret operations (Get, Create, Update) around secretName and related
helpers, and change any helper signatures that currently rely on the global
K8sClient to accept and use reqK8s instead so all connect/status/disconnect
secret reads/writes go through the per-request client.
- Around line 54-58: The handler currently maps any error from
ValidateCodeRabbitAPIKey to a 400; change it to distinguish authentication
failures from transient/provider errors by making ValidateCodeRabbitAPIKey
return a sentinel error (e.g., ErrInvalidCodeRabbitAPIKey) or wrap errors so you
can use errors.Is(err, ErrInvalidCodeRabbitAPIKey) in the coderabbit_auth.go
handler; if errors.Is(...) respond with 400 and the existing message, otherwise
respond with a 5xx (e.g., 502/503) indicating CodeRabbit service/unavailable and
include the full error in the server log (log.Printf) for debugging. Ensure you
update ValidateCodeRabbitAPIKey signature/implementation and the handler check
to use the sentinel/unwrap logic (ValidateCodeRabbitAPIKey,
ErrInvalidCodeRabbitAPIKey) and log the underlying error details when returning
a 5xx.
In `@components/backend/handlers/runtime_credentials.go`:
- Around line 581-597: Replace the manual RBAC/userID logic with a call to
enforceCredentialRBAC to ensure the same RBAC path is used here (instead of
treating an empty authenticatedUserID as a BOT token); then call
GetCodeRabbitCredentials only after enforceCredentialRBAC allows access. Also
when GetCodeRabbitCredentials returns an error, preserve the “not configured”
path by detecting the Secret-not-found condition (e.g., k8s API IsNotFound / the
same check used elsewhere) and return the same not-configured response used by
other handlers, otherwise log and return the generic 500 error as before.
In `@scripts/pre-commit/coderabbit-review.sh`:
- Around line 32-45: The script currently treats transient errors by grepping
OUTPUT for "timeout", but when the `timeout` command kills the process it
returns exit code 124 and produces no stdout message, so timeouts still block
commits; update the logic that runs after EXIT_CODE is set (using variables
OUTPUT and EXIT_CODE from the `timeout 300 "$CR" ...` call) to explicitly treat
EXIT_CODE==124 as a transient timeout (echo a transient message and exit 0)
before the existing grep-based checks.
---
Nitpick comments:
In @.gitignore:
- Line 143: Remove the duplicate .worktrees/ entry from .gitignore: locate the
repeated ignore rule ".worktrees/" and delete the later occurrence so only one
.worktrees/ line remains in the file to avoid redundant entries and confusion.
🪄 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: 43e877cc-8829-4a1d-9a0c-65f00a46779e
📒 Files selected for processing (24)
.gitignore.pre-commit-config.yamlcomponents/backend/handlers/coderabbit_auth.gocomponents/backend/handlers/coderabbit_auth_test.gocomponents/backend/handlers/integration_validation.gocomponents/backend/handlers/integrations_status.gocomponents/backend/handlers/runtime_credentials.gocomponents/backend/routes.gocomponents/backend/tests/constants/labels.gocomponents/frontend/src/app/api/auth/coderabbit/connect/route.tscomponents/frontend/src/app/api/auth/coderabbit/disconnect/route.tscomponents/frontend/src/app/api/auth/coderabbit/status/route.tscomponents/frontend/src/app/api/auth/coderabbit/test/route.tscomponents/frontend/src/app/integrations/IntegrationsClient.tsxcomponents/frontend/src/app/projects/[name]/sessions/[sessionName]/components/settings/__tests__/integrations-panel.test.tsxcomponents/frontend/src/app/projects/[name]/sessions/[sessionName]/components/settings/integrations-panel.tsxcomponents/frontend/src/components/coderabbit-connection-card.tsxcomponents/frontend/src/services/api/coderabbit-auth.tscomponents/frontend/src/services/api/integrations.tscomponents/frontend/src/services/queries/use-coderabbit.tscomponents/runners/ambient-runner/ambient_runner/platform/auth.pydocs/superpowers/plans/2026-04-01-coderabbit-integration.mddocs/superpowers/specs/2026-04-01-coderabbit-integration-design.mdscripts/pre-commit/coderabbit-review.sh
| // Validate API key with CodeRabbit | ||
| if err := ValidateCodeRabbitAPIKey(c.Request.Context(), req.APIKey); err != nil { | ||
| log.Printf("Failed to validate CodeRabbit API key for user %s: %v", userID, err) | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid CodeRabbit API key"}) | ||
| return |
There was a problem hiding this comment.
Don’t report provider outages as “invalid API key”
The validator in components/backend/handlers/integration_validation.go can fail for network/provider reasons, but Line 57 maps every error to 400 Invalid CodeRabbit API key. A transient timeout or upstream failure will reject a valid key and tell the user to rotate it. Reserve 400 for actual auth failures and return a retryable 5xx for upstream errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/backend/handlers/coderabbit_auth.go` around lines 54 - 58, The
handler currently maps any error from ValidateCodeRabbitAPIKey to a 400; change
it to distinguish authentication failures from transient/provider errors by
making ValidateCodeRabbitAPIKey return a sentinel error (e.g.,
ErrInvalidCodeRabbitAPIKey) or wrap errors so you can use errors.Is(err,
ErrInvalidCodeRabbitAPIKey) in the coderabbit_auth.go handler; if errors.Is(...)
respond with 400 and the existing message, otherwise respond with a 5xx (e.g.,
502/503) indicating CodeRabbit service/unavailable and include the full error in
the server log (log.Printf) for debugging. Ensure you update
ValidateCodeRabbitAPIKey signature/implementation and the handler check to use
the sentinel/unwrap logic (ValidateCodeRabbitAPIKey, ErrInvalidCodeRabbitAPIKey)
and log the underlying error details when returning a 5xx.
| secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{}) | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| // Create Secret | ||
| secret = &corev1.Secret{ | ||
| ObjectMeta: v1.ObjectMeta{ | ||
| Name: secretName, | ||
| Namespace: Namespace, | ||
| Labels: map[string]string{ | ||
| "app": "ambient-code", | ||
| "ambient-code.io/provider": "coderabbit", | ||
| }, | ||
| }, | ||
| Type: corev1.SecretTypeOpaque, | ||
| Data: map[string][]byte{}, | ||
| } | ||
| if _, cerr := K8sClient.CoreV1().Secrets(Namespace).Create(ctx, secret, v1.CreateOptions{}); cerr != nil && !errors.IsAlreadyExists(cerr) { | ||
| return fmt.Errorf("failed to create Secret: %w", cerr) | ||
| } | ||
| // Fetch again to get resourceVersion | ||
| secret, err = K8sClient.CoreV1().Secrets(Namespace).Get(ctx, secretName, v1.GetOptions{}) |
There was a problem hiding this comment.
Secret reads/writes are still using the backend service account
The handlers fetch request-scoped clients with GetK8sClientsForRequest(c), but the actual Secret operations on Lines 153, 169, 173, 192, 211, 237, and 251 all go through the package-global K8sClient. That bypasses per-request RBAC for connect/status/disconnect. Pass reqK8s into these helpers and use it for the Secret operations.
As per coding guidelines, User-facing API ops MUST use GetK8sClientsForRequest(c), never the backend service account.
Also applies to: 192-196, 211-213, 237-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/backend/handlers/coderabbit_auth.go` around lines 153 - 173, The
Secret create/read/update calls are using the package-global K8sClient (e.g.,
K8sClient.CoreV1().Secrets(...)) which bypasses per-request RBAC; update the
handlers in coderabbit_auth.go to use the request-scoped client returned by
GetK8sClientsForRequest(c) (e.g., reqK8s) for all Secret operations (Get,
Create, Update) around secretName and related helpers, and change any helper
signatures that currently rely on the global K8sClient to accept and use reqK8s
instead so all connect/status/disconnect secret reads/writes go through the
per-request client.
| // Verify authenticated user owns this session (RBAC: prevent accessing other users' credentials) | ||
| // Note: BOT_TOKEN (session ServiceAccount) won't have userID in context, which is fine - | ||
| // BOT_TOKEN is already scoped to this specific session via RBAC | ||
| authenticatedUserID := c.GetString("userID") | ||
| if authenticatedUserID != "" && authenticatedUserID != userID { | ||
| log.Printf("RBAC violation: user %s attempted to access credentials for session owned by %s", authenticatedUserID, userID) | ||
| c.JSON(http.StatusForbidden, gin.H{"error": "Access denied: session belongs to different user"}) | ||
| return | ||
| } | ||
| // If authenticatedUserID is empty, this is likely BOT_TOKEN (session-scoped ServiceAccount) | ||
| // which is allowed because it's already restricted to this session via K8s RBAC | ||
|
|
||
| creds, err := GetCodeRabbitCredentials(c.Request.Context(), userID) | ||
| if err != nil { | ||
| log.Printf("Failed to get CodeRabbit credentials for user %s: %v", userID, err) | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get CodeRabbit credentials"}) | ||
| return |
There was a problem hiding this comment.
Route this through enforceCredentialRBAC
Line 584 treats a missing userID as if it were a session BOT token, which is weaker than the other session credential handlers in this file and ignores the X-Runner-Current-User path. That can fall through to the session owner’s API key when middleware does not populate userID. The same block on Lines 593-597 also returns 500 when the shared coderabbit-credentials Secret does not exist yet, even though that case should be “not configured”. Reuse enforceCredentialRBAC(...) here and preserve the not-configured path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/backend/handlers/runtime_credentials.go` around lines 581 - 597,
Replace the manual RBAC/userID logic with a call to enforceCredentialRBAC to
ensure the same RBAC path is used here (instead of treating an empty
authenticatedUserID as a BOT token); then call GetCodeRabbitCredentials only
after enforceCredentialRBAC allows access. Also when GetCodeRabbitCredentials
returns an error, preserve the “not configured” path by detecting the
Secret-not-found condition (e.g., k8s API IsNotFound / the same check used
elsewhere) and return the same not-configured response used by other handlers,
otherwise log and return the generic 500 error as before.
| OUTPUT=$(timeout 300 "$CR" review --type uncommitted --prompt-only 2>&1) || EXIT_CODE=$? | ||
| EXIT_CODE=${EXIT_CODE:-0} | ||
|
|
||
| echo "$OUTPUT" | ||
|
|
||
| if [ "$EXIT_CODE" -eq 0 ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Rate limits and network errors should warn, not block | ||
| if echo "$OUTPUT" | grep -qi "rate limit\|network\|timeout\|connection"; then | ||
| echo "CodeRabbit: transient error (see above) — not blocking commit" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Timeout exit code not handled—actual timeouts will block commits.
When timeout kills the process after 300s, it returns exit code 124 but adds no message to stdout. The grep at line 42 checks $OUTPUT for "timeout" string, which won't be present. This means genuine timeouts block commits despite the stated intent.
🛠️ Proposed fix: check exit code 124
OUTPUT=$(timeout 300 "$CR" review --type uncommitted --prompt-only 2>&1) || EXIT_CODE=$?
EXIT_CODE=${EXIT_CODE:-0}
echo "$OUTPUT"
if [ "$EXIT_CODE" -eq 0 ]; then
exit 0
fi
+# Timeout command returns 124 when it kills the child
+if [ "$EXIT_CODE" -eq 124 ]; then
+ echo "CodeRabbit: review timed out after 5 minutes — not blocking commit"
+ exit 0
+fi
+
# Rate limits and network errors should warn, not block
if echo "$OUTPUT" | grep -qi "rate limit\|network\|timeout\|connection"; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/pre-commit/coderabbit-review.sh` around lines 32 - 45, The script
currently treats transient errors by grepping OUTPUT for "timeout", but when the
`timeout` command kills the process it returns exit code 124 and produces no
stdout message, so timeouts still block commits; update the logic that runs
after EXIT_CODE is set (using variables OUTPUT and EXIT_CODE from the `timeout
300 "$CR" ...` call) to explicitly treat EXIT_CODE==124 as a transient timeout
(echo a transient message and exit 0) before the existing grep-based checks.
markturansky
left a comment
markturansky
left a comment
There was a problem hiding this comment.
🐰 CodeRabbit Integration Review
tl;dr: This is a chef's kiss implementation of a full-stack integration. Clean patterns, comprehensive tests, thoughtful error handling, and excellent documentation. A few notes below, but this is production-ready.
The Good Stuff ✨
1. Architecture is 💯
The flow is beautifully simple: API key → K8s Secret → Runtime injection → Pre-commit hook. No overengineering, no unnecessary abstraction. The credential lifecycle follows the exact same pattern as GitHub/GitLab, which means future devs won't need to learn new patterns.
2. Security-conscious design
- API key validation before storage (line 54 in
coderabbit_auth.go) - Proper RBAC checks on runtime credential fetch
- Per-user secret isolation (no cross-contamination)
- Conflict retry logic (3 attempts) on K8s Secret updates
writeOnly: truesemantics implied by one-way storage
3. Graceful degradation everywhere
The pre-commit hook is a masterclass in "fail open when appropriate":
# No CLI? Skip gracefully
# No auth? Skip gracefully
# Rate limit? Warn, don't block
# Network error? Warn, don't block
# Actual findings? BLOCK 🚫This is the right balance between "enforce quality" and "don't break people's workflows when external services hiccup."
4. Test coverage is excellent
7 new Ginkgo tests covering:
- Valid/invalid API keys
- Connect/disconnect flows
- Per-user isolation
- RBAC enforcement on runtime fetch
- Frontend integration panel updated
The test mocking pattern (replacing ValidateCodeRabbitAPIKey with a test stub) is clean and avoids hitting real APIs.
5. Documentation is A+
Both the design spec and implementation plan are detailed, opinionated, and actionable. The "Non-Goals" section is especially valuable — it sets clear boundaries and prevents scope creep.
Questions & Observations 🤔
1. Pre-commit hook timeout: 5 minutes
OUTPUT=$(timeout 300 "$CR" review --type uncommitted --prompt-only 2>&1)5 minutes is generous! Is this based on observed CodeRabbit CLI behavior on large diffs? Just curious if we've seen it take that long in practice, or if this is "better safe than sorry."
Why it matters: Long-running pre-commit hooks can frustrate developers. If CodeRabbit typically finishes in <30s, we might want to tune this down and rely on the transient error handling.
2. API key validation endpoint
req.Header.Set("Authorization", "Bearer "+apiKey)
resp, err := client.Do(req)
return resp.StatusCode == http.StatusOK, nilThe validation hits /api/v1/health with the API key. Does CodeRabbit's health endpoint actually validate the key, or does it just return 200 regardless? If it's the latter, we might want to use a different endpoint (like /api/v1/user or similar) that requires valid auth.
Why it matters: If the health endpoint doesn't check the key, users could "successfully connect" with invalid keys, then get cryptic failures in sessions.
3. Secret storage: cluster-scoped, all users in one Secret
const secretName = "coderabbit-credentials"
secret.Data[creds.UserID] = bThis stores all users' keys in a single K8s Secret, keyed by userID. This is consistent with the existing pattern (good!), but as the user base grows, this Secret could get large.
Potential future optimization: If you hit hundreds of users, consider sharding (e.g., coderabbit-credentials-{hash(userID) % 10}) or switching to per-user Secrets. Not urgent, but worth noting for scale planning.
4. Runner credential fetch: no caching
The runner fetches CodeRabbit credentials at session startup via:
coderabbit_creds = await fetch_coderabbit_credentials(context)This is synchronous with populate_runtime_credentials(), which already fetches GitHub, GitLab, Jira, Google concurrently. Perfect!
But if the credential changes mid-session (user disconnects/reconnects), the session won't pick it up until restart. Is this intentional?
Why it matters: Probably fine — credentials rarely change mid-session, and forcing a restart is reasonable. Just wanted to confirm this is the intended behavior.
5. Pre-commit hook: always_run: true
- id: coderabbit-review
always_run: true
pass_filenames: falseThis runs the hook even when there are no staged files, but the script itself short-circuits:
if git diff --cached --quiet; then
exit 0
fiWhy not set always_run: false in the config and remove the check from the script? Minor, but curious about the reasoning.
6. Error handling: distinguishing transient vs. permanent failures
if echo "$OUTPUT" | grep -qi "rate limit\|network\|timeout\|connection"; then
echo "CodeRabbit: transient error (see above) — not blocking commit"
exit 0
fiThis grep pattern is clever, but it's a heuristic. If CodeRabbit's error messages change, this could miss transient errors (and block commits unnecessarily) or catch false positives (and let through failures that should block).
Suggestion: Consider having the CodeRabbit CLI return specific exit codes for different failure types (e.g., 2 for transient, 1 for findings, 0 for success). Then you can switch on $EXIT_CODE instead of grepping output. (This might require a CodeRabbit CLI feature request.)
Nitpicks 🔬
1. Naming inconsistency: credentials vs. credential
async def fetch_coderabbit_credentials(context: RunnerContext) -> dict:vs.
async def fetch_gitlab_token(context: RunnerContext) -> str:GitLab returns a single token (singular), CodeRabbit returns credentials (plural, but it's just an API key). Not a blocker, just noticed the pattern variance. Might be cleaner to call it fetch_coderabbit_api_key or fetch_coderabbit_credential (singular).
2. Unused return value in validation
valid, err := ValidateCodeRabbitAPIKey(c.Request.Context(), req.APIKey)The function signature returns (bool, error), but in ConnectCodeRabbit you only check err:
if err := ValidateCodeRabbitAPIKey(c.Request.Context(), req.APIKey); err != nil {This works because the function returns (false, err) on failure, but the bool return is redundant if you're only checking err. Consider simplifying to func ValidateCodeRabbitAPIKey(ctx, key) error.
3. Frontend: API client typing
export async function connectCodeRabbit(data: CodeRabbitConnectRequest): Promise<void>The API returns {"message": "CodeRabbit connected successfully"}, but the TS return type is void. Should probably be Promise<{message: string}> if you want type safety on the response. (Though if you're not using the response, void is fine.)
The Verdict 🎯
This PR is ready to ship. The architecture is sound, the error handling is thoughtful, the tests are comprehensive, and the docs are excellent. The questions above are mostly curiosity + future-proofing notes — none are blockers.
What I especially appreciate:
- You didn't overcomplicate it. This could have been 5x larger with "features" nobody asked for.
- The pre-commit hook fail-open logic is production-tested wisdom, not naive optimism.
- The implementation plan reads like a cookbook — anyone could follow it.
- Consistent patterns with existing integrations = low cognitive overhead.
Ship it! 🚢🐰
P.S. The irony of CodeRabbit reviewing its own integration PR is not lost on me. I see CodeRabbit left a comment too — curious if it found anything! 😄
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py`:
- Around line 395-403: The CodeRabbit credential block currently swallows
exceptions and leaves CODERABBIT_API_KEY set; change the
fetch_coderabbit_credentials handling so exceptions are re-raised or propagated
into the existing auth_failures flow instead of converting them to
logger.warning, and ensure the injected secret is removed by
clear_runtime_credentials: after setting os.environ["CODERABBIT_API_KEY"] in the
fetch_coderabbit_credentials success path, register or delete that key in
clear_runtime_credentials (or call a helper to unset CODERABBIT_API_KEY at the
end of the turn); update the try/except around fetch_coderabbit_credentials to
propagate errors into the auth_failures handling rather than suppressing them
with logger.warning.
🪄 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: 17787d24-7b99-4463-8f83-f09e82ccb240
📒 Files selected for processing (3)
components/backend/handlers/integration_validation.gocomponents/backend/routes.gocomponents/runners/ambient-runner/ambient_runner/platform/auth.py
🚧 Files skipped from review as they are similar to previous changes (2)
- components/backend/routes.go
- components/backend/handlers/integration_validation.go
| # CodeRabbit credentials | ||
| try: | ||
| coderabbit_creds = await fetch_coderabbit_credentials(context) | ||
| if coderabbit_creds.get("apiKey"): | ||
| os.environ["CODERABBIT_API_KEY"] = coderabbit_creds["apiKey"] | ||
| logger.info("Updated CodeRabbit API key in environment") | ||
| except Exception as e: | ||
| logger.warning(f"Failed to refresh CodeRabbit credentials: {e}") | ||
|
|
There was a problem hiding this comment.
Do not swallow CodeRabbit auth failures, and clear the injected API key after each turn.
Line 401/Line 402 currently suppress auth failures into a warning, so CodeRabbit can fail silently instead of joining the existing auth_failures path. Also, Line 399 injects a secret env var that is not removed by clear_runtime_credentials (Line 445 onward), leaving CODERABBIT_API_KEY resident longer than intended.
Proposed fix
@@
# CodeRabbit credentials
try:
coderabbit_creds = await fetch_coderabbit_credentials(context)
if coderabbit_creds.get("apiKey"):
os.environ["CODERABBIT_API_KEY"] = coderabbit_creds["apiKey"]
logger.info("Updated CodeRabbit API key in environment")
+ except PermissionError as e:
+ logger.warning(f"Failed to refresh CodeRabbit credentials: {e}")
+ auth_failures.append(str(e))
except Exception as e:
logger.warning(f"Failed to refresh CodeRabbit credentials: {e}")
@@
for key in [
"GITHUB_TOKEN",
"GITLAB_TOKEN",
"JIRA_API_TOKEN",
"JIRA_URL",
"JIRA_EMAIL",
"USER_GOOGLE_EMAIL",
+ "CODERABBIT_API_KEY",
]:As per coding guidelines, “Flag only errors, security risks, or functionality-breaking problems.” and “Check subprocess handling, timeout management, and that secrets are not logged.”
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 401-401: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py` around
lines 395 - 403, The CodeRabbit credential block currently swallows exceptions
and leaves CODERABBIT_API_KEY set; change the fetch_coderabbit_credentials
handling so exceptions are re-raised or propagated into the existing
auth_failures flow instead of converting them to logger.warning, and ensure the
injected secret is removed by clear_runtime_credentials: after setting
os.environ["CODERABBIT_API_KEY"] in the fetch_coderabbit_credentials success
path, register or delete that key in clear_runtime_credentials (or call a helper
to unset CODERABBIT_API_KEY at the end of the turn); update the try/except
around fetch_coderabbit_credentials to propagate errors into the auth_failures
handling rather than suppressing them with logger.warning.
2 participants
Summary
CODERABBIT_API_KEYso thecoderabbitCLI can run local reviews.coderabbit.yamlconfig and pre-commit hook for enforcing reviews at commit timeBackend
coderabbit-credentials)Frontend
Runner
CODERABBIT_API_KEYenv varPre-commit / Config
.coderabbit.yamlwith project-specific review rules, path instructions, custom pre-merge checkscoderabbit-review.shpre-commit hook with rate limit handling and 5-minute timeoutTests
Test plan
go build ./...)npm run build)CODERABBIT_API_KEYinjected in session podcoderabbit reviewon commitFollow-up (separate PRs)
pre-commit installto enabled when CodeRabbit integration is configuredcoderabbitbinary to runner Dockerfile🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
Documentation