Merged
Conversation
webpiratt
requested changes
Feb 25, 2026
Contributor
webpiratt
left a comment
webpiratt
left a comment
There was a problem hiding this comment.
Thanks, please review comments
4others
force-pushed
the
feat/deploy-k8s
branch
from
4372be3 to
5adb0ce
Compare
WalkthroughReplaces API-key based CoinGecko config with a proxy-only client, removes related env variable and deployment scripts, adds multi-stage Docker build and Kubernetes manifests, introduces GitHub Actions workflows for image build/push and dev/prod deployments, and adds a /healthz endpoint to the server. Changes
🚥 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. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/coingecko/client_test.go (1)
8-120:⚠️ Potential issue | 🟠 MajorGate these tests as integration tests to prevent CI flakiness.
These tests make unconditional network calls to live CoinGecko endpoints. They lack the skip mechanism already used in
internal/blockchair/client_test.go. During CI,go test ./...runs without integration test environment variables, causing these tests to fail if the network is unavailable, rate-limited, or the API changes.Apply the
skipUnlessIntegrationpattern already established in blockchair tests:Suggested patch
import ( "context" + "os" "testing" ) +func skipUnlessIntegration(t *testing.T) { + t.Helper() + if os.Getenv("COINGECKO_TEST") != "1" { + t.Skip("set COINGECKO_TEST=1 to run CoinGecko integration tests") + } +} + func TestSearch(t *testing.T) { + skipUnlessIntegration(t) c := NewClient() @@ func TestCoinDetail_Token(t *testing.T) { + skipUnlessIntegration(t) c := NewClient() @@ func TestCoinDetail_NativeAsset(t *testing.T) { + skipUnlessIntegration(t) c := NewClient() @@ func TestSearch_Cache(t *testing.T) { + skipUnlessIntegration(t) c := NewClient() @@ func TestCoinDetail_Cache(t *testing.T) { + skipUnlessIntegration(t) c := NewClient()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/coingecko/client_test.go` around lines 8 - 120, These tests perform live CoinGecko network calls and should be gated as integration tests; add the same skipUnlessIntegration(t) guard used in internal/blockchair tests at the top of each test function (TestSearch, TestCoinDetail_Token, TestCoinDetail_NativeAsset, TestSearch_Cache, TestCoinDetail_Cache) so they call skipUnlessIntegration(t) before making any client calls to avoid CI flakiness when integration tests are not enabled.
🧹 Nitpick comments (4)
Dockerfile (1)
5-5: Use--no-install-recommendsfor apt installs.This reduces image size and unnecessary package surface.
Suggested patch
-RUN apt-get update && apt-get install -y clang lld wget +RUN apt-get update && apt-get install -y --no-install-recommends clang lld wget && rm -rf /var/lib/apt/lists/* -RUN apt-get update && apt-get install -y \ +RUN apt-get update && apt-get install -y --no-install-recommends \ ca-certificates \ && rm -rf /var/lib/apt/lists/*Also applies to: 29-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 5, Update the apt-get install RUN lines to include --no-install-recommends to avoid pulling recommended packages; specifically modify the RUN commands that call "apt-get install -y clang lld wget" (and the other install blocks around lines 29-31) to add --no-install-recommends after apt-get install and keep apt-get update && apt-get install -y ... && rm -rf /var/lib/apt/lists/* pattern to minimize image size..github/workflows/sync-main-to-dev.yaml (1)
8-13: Add workflow concurrency to prevent push races.Back-to-back pushes to
maincan overlap and both try to pushdev, causing avoidable non-fast-forward failures.Suggested patch
name: Sync main to dev on: push: branches: - main + +concurrency: + group: sync-main-to-dev + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync-main-to-dev.yaml around lines 8 - 13, The workflow lacks a concurrency policy which allows overlapping runs of the sync-main-to-dev job ("sync-main-to-dev") to race and cause non-fast-forward failures; add a concurrency stanza to the workflow (at the workflow root or job level for the "sync-main-to-dev" job) that defines a stable group name (e.g., based on repo and branch like "sync-main-to-dev-${{ github.ref }}") and set cancel-in-progress: true so only one push-run proceeds at a time and in-flight runs are cancelled to prevent push races.deploy/prod/01_config.yaml (1)
6-6: Consider RPC failover for production resilience.Using one public Ethereum RPC endpoint creates a single upstream failure/rate-limit point. Consider managed RPC with SLA or a failover strategy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/prod/01_config.yaml` at line 6, The production config currently hardcodes a single RPC endpoint via the eth-rpc-url key which is a single point of failure; change this to support managed RPC or a failover list by replacing eth-rpc-url with a configurable list (e.g., eth-rpc-urls) or an env var that accepts multiple endpoints and update the app code that reads eth-rpc-url to iterate/failover with retries and backoff (or use a hosted RPC provider with SLA) so the client code (where eth-rpc-url is consumed) will try the next endpoint on error or rate-limit and log which endpoint failed.deploy/01_server.yaml (1)
37-37: Avoid mutable:latestfor deploy images.At Line 37,
:latestmakes rollouts non-deterministic and harder to roll back. Prefer an immutable tag (or digest) produced by CI.♻️ Example change
- image: ghcr.io/vultisig/mcp/mcp-server:latest + image: ghcr.io/vultisig/mcp/mcp-server:${IMAGE_TAG}Or pin directly by digest:
- image: ghcr.io/vultisig/mcp/mcp-server:latest + image: ghcr.io/vultisig/mcp/mcp-server@sha256:<digest>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/01_server.yaml` at line 37, Replace the mutable image tag "ghcr.io/vultisig/mcp/mcp-server:latest" with an immutable, CI-produced identifier: either a build-specific tag variable (e.g., injected CI variable like MCP_SERVER_TAG used in the Deployment spec) or the image digest (ghcr.../mcp-server@sha256:<digest>); update the deployment template so the image field in the Server Deployment (image: ghcr.io/vultisig/mcp/mcp-server) is parameterized to accept the CI-generated tag/digest and ensure the CI pipeline writes the exact tag/digest into the manifest or replaces the variable at deploy time for deterministic rollouts and rollbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-push-image.yaml:
- Line 75: The workflow currently disables build provenance with the setting
"provenance: false", which removes provenance attestations; change that setting
to enable provenance (set "provenance" to true or remove the explicit false so
the default enables it) in the build-push-image workflow to restore image
provenance attestations and improve supply-chain traceability.
In @.github/workflows/deploy-prod.yaml:
- Around line 31-37: The "Set GHCR packages public" workflow step is calling a
non-existent PATCH endpoint (gh api -X PATCH
"/orgs/${owner}/packages/container/${repo}%2Fmcp-server/visibility") which
silently fails; remove this step or replace it after researching a valid
approach for changing GHCR package visibility (e.g., manual UI change or an
official API/tool) and update the workflow accordingly—look for the step named
"Set GHCR packages public" and the gh api call string in the job to locate and
remove/replace it.
In `@deploy/01_server.yaml`:
- Around line 32-67: The pod currently has no securityContext; add a pod-level
securityContext (e.g., runAsNonRoot: true, runAsUser: 1000, runAsGroup: 1000,
fsGroup: 1000) and a container-level securityContext on the container named
"server" to harden runtime (set runAsNonRoot: true, allowPrivilegeEscalation:
false, privileged: false, readOnlyRootFilesystem: true, drop all capabilities,
and seccompProfile: { type: RuntimeDefault } or similar). Ensure these fields
are applied under the same spec that contains the "server" container so
readinessProbe/livenessProbe continue to work and resource requests/limits
remain unchanged. Validate the cluster supports the chosen UID/group and seccomp
settings before merging.
- Around line 43-47: The environment variable name used in the deployment
override is incorrect: replace the env var key ETH_RPC_URL with the
config-contract-compliant name EVM_ETHEREUM_URL so the app picks up the
ConfigMap value (keep the valueFrom configMapKeyRef key: eth-rpc-url unchanged);
update any other deployment or manifest occurrences of ETH_RPC_URL to
EVM_ETHEREUM_URL to ensure consistent config binding.
In `@Dockerfile`:
- Around line 27-37: Create and switch to a non-root runtime user in the
Dockerfile: add steps after copying artifacts to create a dedicated user/group
(e.g., appuser), chown the application files and the /usr/local/lib/dkls path
(referencing the COPY --from=builder /app/main and COPY --from=builder
/usr/local/lib/dkls lines and WORKDIR /app), and then set USER to that non-root
user before the final image is produced; keep the existing ENV LD_LIBRARY_PATH
line but ensure the non-root user has permission to read those libraries.
- Around line 7-13: The Dockerfile is fetching go-wrappers from
refs/heads/master.tar.gz which is mutable; change the wget URL to a pinned
immutable ref (a specific release tag or commit archive URL for go-wrappers) and
add a SHA256 checksum verification step before extracting: download the tarball
to a temp file, compute and compare its SHA256 against the expected value, fail
the build if it doesn't match, then extract and proceed (update the RUN command
that currently references master.tar.gz and the subsequent tar -xzf and cd
go-wrappers-master steps to use the pinned filename/dir). Ensure the checksum is
stored in the Dockerfile or ARG and the extraction path/name (e.g.,
go-wrappers-<tag>) is updated accordingly so the subsequent cp and
includes/linux-${TARGETARCH} checks still point to the correct directory.
---
Outside diff comments:
In `@internal/coingecko/client_test.go`:
- Around line 8-120: These tests perform live CoinGecko network calls and should
be gated as integration tests; add the same skipUnlessIntegration(t) guard used
in internal/blockchair tests at the top of each test function (TestSearch,
TestCoinDetail_Token, TestCoinDetail_NativeAsset, TestSearch_Cache,
TestCoinDetail_Cache) so they call skipUnlessIntegration(t) before making any
client calls to avoid CI flakiness when integration tests are not enabled.
---
Nitpick comments:
In @.github/workflows/sync-main-to-dev.yaml:
- Around line 8-13: The workflow lacks a concurrency policy which allows
overlapping runs of the sync-main-to-dev job ("sync-main-to-dev") to race and
cause non-fast-forward failures; add a concurrency stanza to the workflow (at
the workflow root or job level for the "sync-main-to-dev" job) that defines a
stable group name (e.g., based on repo and branch like "sync-main-to-dev-${{
github.ref }}") and set cancel-in-progress: true so only one push-run proceeds
at a time and in-flight runs are cancelled to prevent push races.
In `@deploy/01_server.yaml`:
- Line 37: Replace the mutable image tag
"ghcr.io/vultisig/mcp/mcp-server:latest" with an immutable, CI-produced
identifier: either a build-specific tag variable (e.g., injected CI variable
like MCP_SERVER_TAG used in the Deployment spec) or the image digest
(ghcr.../mcp-server@sha256:<digest>); update the deployment template so the
image field in the Server Deployment (image: ghcr.io/vultisig/mcp/mcp-server) is
parameterized to accept the CI-generated tag/digest and ensure the CI pipeline
writes the exact tag/digest into the manifest or replaces the variable at deploy
time for deterministic rollouts and rollbacks.
In `@deploy/prod/01_config.yaml`:
- Line 6: The production config currently hardcodes a single RPC endpoint via
the eth-rpc-url key which is a single point of failure; change this to support
managed RPC or a failover list by replacing eth-rpc-url with a configurable list
(e.g., eth-rpc-urls) or an env var that accepts multiple endpoints and update
the app code that reads eth-rpc-url to iterate/failover with retries and backoff
(or use a hosted RPC provider with SLA) so the client code (where eth-rpc-url is
consumed) will try the next endpoint on error or rate-limit and log which
endpoint failed.
In `@Dockerfile`:
- Line 5: Update the apt-get install RUN lines to include
--no-install-recommends to avoid pulling recommended packages; specifically
modify the RUN commands that call "apt-get install -y clang lld wget" (and the
other install blocks around lines 29-31) to add --no-install-recommends after
apt-get install and keep apt-get update && apt-get install -y ... && rm -rf
/var/lib/apt/lists/* pattern to minimize image size.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.env.example.github/workflows/build-push-image.yaml.github/workflows/deploy-dev.yaml.github/workflows/deploy-prod.yaml.github/workflows/deploy.yml.github/workflows/sync-main-to-dev.yamlCLAUDE.mdDockerfilecmd/mcp-server/main.godeploy/01_server.yamldeploy/dev/01_config.yamldeploy/dev/02_ingress.yamldeploy/prod/01_config.yamldeploy/prod/02_ingress.yamlinternal/coingecko/client.gointernal/coingecko/client_test.gointernal/config/config.goscripts/deploy.sh
💤 Files with no reviewable changes (3)
- .env.example
- .github/workflows/deploy.yml
- scripts/deploy.sh
webpiratt
reviewed
Feb 26, 2026
4others
force-pushed
the
feat/deploy-k8s
branch
from
5adb0ce to
7860f98
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/deploy-prod.yaml (1)
81-85: Optional cleanup: avoid re-mutating manifest in deploy job.
update-manifestsalready rewrites and pushesdeploy/01_server.yaml; repeatingsedhere is redundant and can be removed to keep one source of truth for tag updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-prod.yaml around lines 81 - 85, Remove the redundant tag-rewrite step in the deploy job that runs the shell snippet updating deploy/01_server.yaml (the step named "Update deployment files" which runs sed to replace mcp-server:${TAG}); since update-manifests already rewrites and pushes deploy/01_server.yaml, delete this sed step (or the entire "Update deployment files" step) so the manifest is only mutated once and the workflow relies on update-manifests as the single source of truth..github/workflows/sync-main-to-dev.yaml (1)
19-26: Consider handling merge conflicts explicitly in the sync step.If
git merge origin/mainconflicts, the job hard-fails. Consider adding explicit conflict handling/logging (or opening a PR fallback) to make the automation behavior clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync-main-to-dev.yaml around lines 19 - 26, The sync step named "Sync main to dev" currently hard-fails on merge conflicts; update the job to detect the failure of the git merge command (the existing git merge origin/main --no-edit) and handle it explicitly by logging the conflict, aborting the merge (git merge --abort), creating a conflict branch (e.g., sync/main-into-dev-conflict-<SHA>), pushing that branch, and opening a PR fallback (using the gh CLI or GitHub API) so conflicts can be resolved manually; ensure the step returns a clear status/log message and only proceeds with git push origin dev when the merge command succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 25: The Dockerfile currently builds a single Go file (RUN go build -o
main cmd/${SERVICE}/main.go); change the build target to the package directory
so the package is compiled (e.g., use ./cmd/${SERVICE}/) — update the RUN
command that invokes go build in the Dockerfile to target ./cmd/${SERVICE}/
(keeping -o main and the SERVICE variable usage) to follow the
cmd/mcp-server/**/*.go guideline and avoid brittle single-file builds.
---
Nitpick comments:
In @.github/workflows/deploy-prod.yaml:
- Around line 81-85: Remove the redundant tag-rewrite step in the deploy job
that runs the shell snippet updating deploy/01_server.yaml (the step named
"Update deployment files" which runs sed to replace mcp-server:${TAG}); since
update-manifests already rewrites and pushes deploy/01_server.yaml, delete this
sed step (or the entire "Update deployment files" step) so the manifest is only
mutated once and the workflow relies on update-manifests as the single source of
truth.
In @.github/workflows/sync-main-to-dev.yaml:
- Around line 19-26: The sync step named "Sync main to dev" currently hard-fails
on merge conflicts; update the job to detect the failure of the git merge
command (the existing git merge origin/main --no-edit) and handle it explicitly
by logging the conflict, aborting the merge (git merge --abort), creating a
conflict branch (e.g., sync/main-into-dev-conflict-<SHA>), pushing that branch,
and opening a PR fallback (using the gh CLI or GitHub API) so conflicts can be
resolved manually; ensure the step returns a clear status/log message and only
proceeds with git push origin dev when the merge command succeeds.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.env.example.github/workflows/build-push-image.yaml.github/workflows/deploy-dev.yaml.github/workflows/deploy-prod.yaml.github/workflows/deploy.yml.github/workflows/sync-main-to-dev.yamlCLAUDE.mdDockerfilecmd/mcp-server/main.godeploy/01_server.yamldeploy/dev/01_config.yamldeploy/dev/02_ingress.yamldeploy/prod/01_config.yamldeploy/prod/02_ingress.yamlinternal/coingecko/client.gointernal/coingecko/client_test.gointernal/config/config.goscripts/deploy.sh
💤 Files with no reviewable changes (3)
- .github/workflows/deploy.yml
- scripts/deploy.sh
- .env.example
🚧 Files skipped from review as they are similar to previous changes (7)
- CLAUDE.md
- internal/config/config.go
- deploy/dev/01_config.yaml
- deploy/dev/02_ingress.yaml
- internal/coingecko/client_test.go
- .github/workflows/deploy-dev.yaml
- internal/coingecko/client.go
webpiratt
approved these changes
Feb 27, 2026
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
3 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
Chores & DevOps