fix(runner): make integrations behave more consistently in session#1686
Merged
markturansky merged 11 commits intoJun 16, 2026
Merged
Conversation
`build_credential_mcp_servers()` only registers the Jira MCP subprocess when `jira` is present in `CREDENTIAL_IDS` (set via RoleBinding resolution). However, `populate_runtime_credentials()` fetches Jira credentials via the session-scoped fallback endpoint and sets JIRA_URL/JIRA_API_TOKEN in the environment regardless of binding state. Add an env-var fallback in `build_mcp_servers()`: if JIRA_URL and JIRA_API_TOKEN are set but no Jira MCP server was added by the credential binding path, register mcp-atlassian from the registry (same pattern Gerrit already uses via GERRIT_CONFIG_PATH). Previously the system prompt always told Claude to ask users to configure Jira/Google when those integrations were needed, even when MCP servers were already loaded and credentials were present in the environment. Claude followed the static instruction and reported integrations as unavailable, requiring a user follow-up to remind it the MCP was loaded. Root cause: MCP_INTEGRATIONS_PROMPT was a static string evaluated at import time, and the system prompt was not rebuilt after the first-run credential refresh that populates env vars like JIRA_URL and JIRA_API_TOKEN. Fixes: - Replace static MCP_INTEGRATIONS_PROMPT with a data-driven _INTEGRATION_REGISTRY covering GitHub, GitLab, Jira, and Google Workspace. Each entry has a detect() callable and mode-keyed prompts. When an integration is configured, Claude is told to use its MCP server/tools; when not, it's told to ask the user to set it up. New integrations can be added by appending a single _Integration entry. - Add _rebuild_system_prompt() to ClaudeBridge and call it alongside _rebuild_mcp_servers() on first run, after credential refresh, so the prompt accurately reflects actual integration state even when the backend was slow on the initial _setup_platform() credential fetch. - Update the Gemini CLI bridge to use _build_integrations_prompt() for the same conditional behavior instead of the old static constant + separate blocks. - Add TestBuildIntegrationsPrompt with parameterized tests for all 4 integrations covering configured/not-configured states and both sidecar and token modes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughReplaces static per-provider MCP/token prompt constants with a registry-driven ChangesDynamic integration prompts, Jira MCP fallback, and Claude first-run rebuild
Possibly related PRs
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py (1)
230-238:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUser-change rebuild is incorrectly gated by per-thread worker existence (credential leakage risk).
At Line 230, rebuild happens only if
get_existing(thread_id)is true. The adapter/MCP config is shared across threads, so a user switch on a new thread can still reuse the previous user’s adapter/mcp env-expanded credentials and stale system prompt.Suggested fix
- elif user_changed and self._session_manager.get_existing(thread_id): - logger.info( - f"User changed for thread={thread_id}, " - "rebuilding MCP servers and adapter with new credentials" - ) - await self._session_manager.destroy(thread_id) - self._rebuild_mcp_servers() - # Force adapter rebuild so ClaudeAgentOptions uses new mcp_servers - self._adapter = None + elif user_changed: + logger.info( + f"User changed for thread={thread_id}, " + "rebuilding MCP servers, prompt, and adapter with new credentials" + ) + if self._session_manager.get_existing(thread_id): + await self._session_manager.destroy(thread_id) + self._rebuild_mcp_servers() + self._rebuild_system_prompt() + # Force adapter rebuild so ClaudeAgentOptions uses updated config + self._adapter = None🤖 Prompt for 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. In `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py` around lines 230 - 238, The user-change rebuild logic at the condition checking user_changed and self._session_manager.get_existing(thread_id) is incorrectly gated by per-thread worker existence. Since the adapter and MCP servers are shared across all threads (not per-thread), removing the specific thread_id from an existing session check will ensure the rebuild happens whenever a user changes, regardless of whether that particular thread_id already has a worker. This prevents the risk of new threads reusing the previous user's stale adapter and environment-expanded credentials. Restructure the condition so that rebuilding the MCP servers and setting self._adapter to None occurs whenever user_changed is true, independent of the per-thread worker existence check.
🤖 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
`@components/runners/ambient-runner/ambient_runner/bridges/gemini_cli/system_prompt.py`:
- Around line 201-203: The prompt builder unconditionally includes
GIT_PUSH_STEPS even when GitHub MCP mode is active, creating conflicting
instructions (MCP mode says use GitHub MCP tools/don't use git push, while
GIT_PUSH_STEPS instructs git/gh flow). Identify where GIT_PUSH_STEPS is being
appended to the sections list and make it conditional: only add GIT_PUSH_STEPS
when GitHub sidecar/MCP mode is not active. Ensure the mode detection uses the
same logic as _build_integrations_prompt() uses to determine whether to emit
MCP-related instructions, so the prompt remains coherent.
In `@components/runners/ambient-runner/ambient_runner/platform/prompts.py`:
- Around line 151-154: The _detect_jira function currently only checks for
JIRA_URL and JIRA_API_TOKEN environment variables, but it should also detect
when Jira is configured via CREDENTIAL_MCP_URLS provider keys. Update the
function to check both the environment variables and the CREDENTIAL_MCP_URLS
configuration (checking for Jira-related provider keys). Return "mcp" if either
configuration method is detected, so that Jira MCP is properly recognized
regardless of how it was configured.
---
Outside diff comments:
In `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py`:
- Around line 230-238: The user-change rebuild logic at the condition checking
user_changed and self._session_manager.get_existing(thread_id) is incorrectly
gated by per-thread worker existence. Since the adapter and MCP servers are
shared across all threads (not per-thread), removing the specific thread_id from
an existing session check will ensure the rebuild happens whenever a user
changes, regardless of whether that particular thread_id already has a worker.
This prevents the risk of new threads reusing the previous user's stale adapter
and environment-expanded credentials. Restructure the condition so that
rebuilding the MCP servers and setting self._adapter to None occurs whenever
user_changed is true, independent of the per-thread worker existence check.
🪄 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: Enterprise
Run ID: 7d15312c-1630-40b8-9be7-8c8a330c906a
📒 Files selected for processing (5)
components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.pycomponents/runners/ambient-runner/ambient_runner/bridges/claude/mcp.pycomponents/runners/ambient-runner/ambient_runner/bridges/gemini_cli/system_prompt.pycomponents/runners/ambient-runner/ambient_runner/platform/prompts.pycomponents/runners/ambient-runner/tests/test_bridge_claude.py
Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>
Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>
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
2 participants
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.
This PR fixes issues with Integrations not being loaded into session consistently. In prior test runs against
main, a configured Jira integration would rarely work often resulting in this type of behavior.Following up to tell the agent that Jira MCP is indeed available and the integration is configured often resulted in it not knowing the integration was even available to it within session:
Following this change Jira (and other integrations) work on the first invocation of the prompt:
Relates to: #1506
Summary by CodeRabbit