Repository navigation
[ROB-3039] GitHub App credentials mcp config - #1717
Merged
Merged
Conversation
Signed-off-by: avi@robusta.dev <avi@robusta.dev>
Signed-off-by: avi@robusta.dev <avi@robusta.dev>
Signed-off-by: avi@robusta.dev <avi@robusta.dev>
Contributor
📂 Previous Runs📜 Run @ ecb5beb (#22941671416)✅ Results of HolmesGPT evalsAutomatically triggered by commit ecb5beb on branch Results of HolmesGPT evals
Benchmark comparison unavailable: No ci-benchmark experiments found Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: No ci-benchmark experiments found Comparison indicators:
📜 Run @ 888e299 (#22843859059)✅ Results of HolmesGPT evalsAutomatically triggered by commit 888e299 on branch Results of HolmesGPT evals
Benchmark comparison unavailable: No ci-benchmark experiments found Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: No ci-benchmark experiments found Comparison indicators:
📜 Run @ 0ee5242 (#22843647858)✅ Results of HolmesGPT evalsAutomatically triggered by commit 0ee5242 on branch Results of HolmesGPT evals
Benchmark comparison unavailable: No ci-benchmark experiments found Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: No ci-benchmark experiments found Comparison indicators:
✅ Results of HolmesGPT evalsAutomatically triggered by commit 9f69409 on branch Results of HolmesGPT evals
Benchmark comparison unavailable: No ci-benchmark experiments found Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: No ci-benchmark experiments found Comparison indicators:
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" Option 3: Add PR labels to include extra evals in automatic regression runs:
Examples: 🏷️ Valid tags
Commands: CLI: |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughSwitches GitHub token management from in-process Python to an in-cluster github-app-mcp image and adds GitHub App authentication as an alternative to PATs. Updates docs, Helm charts, and tests; removes the Python GitHub App token manager and its tests; routes MCP traffic to in-cluster /mcp with streamable-http for the App flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Holmes as Holmes CLI/Pod
participant MCP as In-cluster MCP Server
participant GitHubMCP as github-app-mcp Image
participant GitHub as GitHub API
Note over GitHubMCP,GitHub: github-app-mcp manages App tokens
GitHubMCP->>GitHubMCP: Read GITHUB_APP_* from Secret
GitHubMCP->>GitHub: POST /app/installations/{id}/access_tokens (with JWT)
GitHub-->>GitHubMCP: Return installation token
GitHubMCP->>MCP: Expose /mcp and use token for proxied requests
loop Periodic refresh
GitHubMCP->>GitHubMCP: Generate new JWT
GitHubMCP->>GitHub: Refresh installation token
GitHub-->>GitHubMCP: New token
end
Holmes->>MCP: Connect to /mcp (streamableHttp)
MCP->>GitHubMCP: Proxy GitHub API operations
GitHubMCP->>GitHub: API requests using managed token
GitHub-->>GitHubMCP: API responses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Contributor
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:5cbd3181
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:5cbd3181 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:5cbd3181
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:5cbd3181
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:5cbd3181
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:5cbd3181 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:5cbd3181
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:5cbd3181Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:5cbd3181 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:5cbd3181Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:5cbd3181 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:5cbd3181 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
Avi-Robusta
changed the title
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/data-sources/builtin-toolsets/github-mcp.md`:
- Around line 306-363: The Deployment example for github-mcp-server is missing
creation of the holmes-github-app Secret and a Service to expose
github-mcp-server, so update the docs to either (a) include YAML snippets that
create the holmes-github-app Secret (containing GITHUB_APP_ID,
GITHUB_APP_INSTALLATION_ID, GITHUB_APP_PRIVATE_KEY) and a ClusterIP Service
named github-mcp-server in the holmes-mcp namespace, or (b) explicitly tell
readers to reuse the PAT CLI example’s Secret and Service steps before adding
the mcp_servers entry in ~/.holmes/config.yaml; reference the Deployment
resource name github-mcp-server, the secret name holmes-github-app, and the
config key mcp_servers.github.url so maintainers know exactly what to add or
point to.
- Around line 7-12: The prerequisites section is misleading for GitHub App users
because the overview lists both "Personal Access Token (PAT)" and "GitHub App"
but the following paragraph implies a PAT is required; update the docs to split
or clarify prerequisites per auth method: under the "Personal Access Token
(PAT)" subsection state explicitly that a PAT is required and how to provide it
to the `github-mcp` image, and under the "GitHub App" subsection remove any PAT
requirement and instead describe the credentials needed for the `github-app-mcp`
image (App ID, private key, installation ID) and how tokens are
generated/rotated by that image so readers using "GitHub App" aren’t directed to
create a PAT.
In `@helm/holmes/templates/mcp-servers/github/deployment.yaml`:
- Around line 65-111: The template currently lets auth.githubApp.secretName
silently override auth.secretName causing image/transport/env mismatches; add a
render-time guard that fails fast if both
.Values.mcpAddons.github.auth.githubApp.secretName and
.Values.mcpAddons.github.auth.secretName are set (e.g. compute a boolean
$hasGitHubApp and call required or fail with a clear message when both are
present) in helm/holmes/templates/mcp-servers/github/deployment.yaml, and apply
the same $hasGitHubApp guard/conditional in
helm/holmes/templates/toolset-config.yaml so the image, args/--outputTransport,
and env blocks stay in sync and misconfiguration is prevented at render time.
In `@helm/holmes/values.yaml`:
- Around line 327-332: Update the GitHub App secret example in values.yaml so it
creates all required keys for the GitHub App flow: include GITHUB_APP_ID,
GITHUB_APP_INSTALLATION_ID and GITHUB_APP_PRIVATE_KEY in the kubectl create
secret generic command (or replace the one-liner with a note pointing to the
detailed example), ensuring the example secret name and namespace remain
consistent with the rest of the chart; edit the block containing the current
example that references GITHUB_APP_ID to either inline a full kubectl create
secret generic ... --from-literal=GITHUB_APP_ID=...
--from-literal=GITHUB_APP_INSTALLATION_ID=...
--from-file=GITHUB_APP_PRIVATE_KEY=... or add an explicit pointer to the
detailed example below so the pod receives all three values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cdec7c2e-d980-4ff3-96a8-24a8b796b10c
📒 Files selected for processing (8)
docs/data-sources/builtin-toolsets/github-mcp.mdhelm/holmes/templates/mcp-servers/github/deployment.yamlhelm/holmes/templates/toolset-config.yamlhelm/holmes/values.yamlholmes/core/toolset_manager.pyholmes/utils/github_app_token_manager.pytests/test_mcp_toolset.pytests/utils/test_github_app_token_manager.py
💤 Files with no reviewable changes (3)
- holmes/core/toolset_manager.py
- tests/utils/test_github_app_token_manager.py
- holmes/utils/github_app_token_manager.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/data-sources/builtin-toolsets/github-mcp.md (2)
320-377:⚠️ Potential issue | 🟡 MinorAdd the Service to the GitHub App CLI example.
This flow points
mcp_servers.github.config.urlatgithub-mcp-server.holmes-mcp.svc.cluster.local:8000/mcp, but the example only creates the Secret and Deployment. Without aService, that DNS name will not resolve, so the CLI example is still incomplete as written.Suggested doc patch
```yaml apiVersion: apps/v1 kind: Deployment metadata: name: github-mcp-server @@ - name: GITHUB_APP_PRIVATE_KEY valueFrom: secretKeyRef: name: holmes-github-app key: GITHUB_APP_PRIVATE_KEY + --- + apiVersion: v1 + kind: Service + metadata: + name: github-mcp-server + namespace: holmes-mcp + spec: + selector: + app: github-mcp-server + ports: + - port: 8000 + targetPort: 8000 + protocol: TCP + name: http ``` + + Apply it to the cluster: + + ```bash + kubectl apply -f github-app-mcp-deployment.yaml + ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data-sources/builtin-toolsets/github-mcp.md` around lines 320 - 377, The documentation is missing a Kubernetes Service for the github-mcp-server Deployment so the DNS github-mcp-server.holmes-mcp.svc.cluster.local:8000 will not resolve; add a Service manifest that selects pods with label app: github-mcp-server and exposes port 8000 (port 8000 -> targetPort 8000, TCP) with the same name (e.g., github-mcp-server) and include instructions to apply the manifest (kubectl apply -f ...) so mcp_servers.github.config.url can reach the MCP server.
7-12:⚠️ Potential issue | 🟡 MinorClarify prerequisites per auth method.
The overview now says PAT and GitHub App are both supported, but the next section still opens by saying a GitHub PAT is required before deployment. That still sends GitHub App users down the wrong path; scope the PAT prerequisite to the PAT flow or split prerequisites by auth method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/data-sources/builtin-toolsets/github-mcp.md` around lines 7 - 12, The prerequisites section currently states a GitHub PAT is required globally; update the docs to scope prerequisites per authentication method by either (A) moving the PAT prerequisite into the PAT flow and labeling it for "Personal Access Token (PAT) / github-mcp image" or (B) splitting the prerequisites into two subsections titled "PAT (github-mcp image)" and "GitHub App (github-app-mcp image)" and listing only the relevant credentials/permissions for each (PAT and scopes for PAT flow; App ID, private key, installation ID, and required permissions for GitHub App flow).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/data-sources/builtin-toolsets/github-mcp.md`:
- Around line 320-377: The documentation is missing a Kubernetes Service for the
github-mcp-server Deployment so the DNS
github-mcp-server.holmes-mcp.svc.cluster.local:8000 will not resolve; add a
Service manifest that selects pods with label app: github-mcp-server and exposes
port 8000 (port 8000 -> targetPort 8000, TCP) with the same name (e.g.,
github-mcp-server) and include instructions to apply the manifest (kubectl apply
-f ...) so mcp_servers.github.config.url can reach the MCP server.
- Around line 7-12: The prerequisites section currently states a GitHub PAT is
required globally; update the docs to scope prerequisites per authentication
method by either (A) moving the PAT prerequisite into the PAT flow and labeling
it for "Personal Access Token (PAT) / github-mcp image" or (B) splitting the
prerequisites into two subsections titled "PAT (github-mcp image)" and "GitHub
App (github-app-mcp image)" and listing only the relevant
credentials/permissions for each (PAT and scopes for PAT flow; App ID, private
key, installation ID, and required permissions for GitHub App flow).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a262d8ec-0230-4781-837b-43f574b74ab9
📒 Files selected for processing (1)
docs/data-sources/builtin-toolsets/github-mcp.md
RoiGlinik
approved these changes
Mar 9, 2026
Avi-Robusta
enabled auto-merge (squash)
March 11, 2026 07:30
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.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests