feat: make Hub MCP self-hosting production-ready#66
Merged
Conversation
✱ Stainless preview buildsThis PR will update the
|
WalkthroughThis PR adds OpenAPI specification serving capabilities, MCP smoke test infrastructure, and configuration for self-hosted deployments. The OpenAPI document is now served at 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/app_test.go`:
- Around line 189-204: Add a new test mirroring
TestNewHTTPServerServesOpenAPIWithoutAuth but targeting "/openapi.yaml" and
ensuring the server behavior when PUBLIC_BASE_URL is unset: unset or clear the
PUBLIC_BASE_URL env var before calling newTestHTTPServer(), create the request
with httptest.NewRequestWithContext for "/openapi.yaml", call
server.Handler.ServeHTTP and assert recorder.Code == http.StatusOK, then assert
the response body contains the expected default public base URL string (use the
same substring checked in the JSON test, e.g. "https://hub.example.com/base" or
the package constant that defines the default base) so the regression for the
YAML endpoint is covered.
In `@cmd/api/app.go`:
- Around line 330-333: If handlers.NewOpenAPIHandler(...) fails in NewApp,
ensure you clean up previously-initialized resources before returning: call the
message manager and river client cleanup methods (e.g., messageManager.Close or
messageManager.Shutdown and riverClient.Close) and gracefully shutdown the
telemetry providers (tracerProvider and meterProvider) using their
Shutdown/ForceFlush APIs with a timeout context, then return the formatted
error; add this cleanup branch immediately before the current "return nil,
fmt.Errorf(...)" so resources initialized earlier in NewApp are not leaked.
In `@internal/api/handlers/openapi_handler.go`:
- Around line 216-227: requestBaseURL currently injects the raw r.Host into the
OpenAPI servers URL which allows host-header injection; update requestBaseURL to
first prefer an explicit PUBLIC_BASE_URL env/config and use that for the
returned base URL, and if PUBLIC_BASE_URL is unset do NOT blindly trust
r.Host—either return a safe localDevelopmentBaseURL only in development or
validate r.Host against an explicit allowlist (implement a helper like
isHostAllowed(host string) used by requestBaseURL) and reject/ignore unallowed
hosts; reference the function requestBaseURL, the variable
localDevelopmentBaseURL and the PUBLIC_BASE_URL configuration when making this
change.
In `@internal/config/config_test.go`:
- Around line 15-21: The tests in internal/config/config_test.go leak the parent
process's PUBLIC_BASE_URL when setPublicBaseURL is false; update the subtest
setup so that before calling Load() you capture the original
os.Getenv("PUBLIC_BASE_URL"), then if setPublicBaseURL is false explicitly unset
the env var (os.Unsetenv) and after the subtest restore the original value (or
clear if it was empty); apply the same change to the other table cases noted
(around the block referenced by lines 95-97) so all subtests deterministically
control PUBLIC_BASE_URL when exercising Load().
In `@Makefile`:
- Around line 96-127: The check-coverage-staged Makefile target is too large and
should be extracted into a small wrapper that calls a dedicated script to avoid
checkmake max-body-length warnings; move the 31-line shell block into a new
executable script (e.g., scripts/check_coverage_staged.sh) that accepts or reads
STAGED_PACKAGES and PRE_COMMIT_COVERAGE_THRESHOLD, performs the same steps
(compute STAGED_GO_FILES, build STAGED_GO_FILE_LIST, run go test to produce
coverage.staged.out, calculate COV with awk against MODULE, and exit non-zero on
failures), and replace the large target body with a one-line Make recipe that
invokes the script (preserving the target name check-coverage-staged and all
original environment variable usage and exit codes).
In `@openapi.yaml`:
- Around line 8-9: The comment points out that the README in openapi.yaml
mentions the wrong environment variable name; replace references to
FORMBRICKS_HUB_BASE_URL with the actual env var used by the runtime/server
config (PUBLIC_BASE_URL) so documentation matches the running configuration and
avoids confusing self-hosted users—update any occurrences of
FORMBRICKS_HUB_BASE_URL in openapi.yaml to PUBLIC_BASE_URL and ensure the
surrounding sentence clarifies that the published JS/TS packages use
PUBLIC_BASE_URL for overriding the Hub base URL.
In `@tests/mcp_smoke_test.go`:
- Around line 375-403: The created feedback record can be left behind if the
intermediate list call fails; wrap the listing and deletion in a try/finally so
deletion is always attempted: call client.feedbackRecords.create(...) as before
to get created (ensure created and created.id exist), then do the list call
inside a try block and in the finally block call await
client.feedbackRecords.delete(created.id) (with a safe existence check and
optional error swallow/log) before returning the result from the async function
run(client).
- Around line 352-365: The test helper failMCPTestf logs raw subprocess stderr
which can leak secrets (e.g. HUB_API_KEY passed into child env); update
failMCPTestf to redact known secret values before calling t.Log by scanning
stderr.lines (from tailWriter) and replacing occurrences of the HUB_API_KEY
value (and any other sensitive env values) with a fixed mask like "[REDACTED]";
ensure you read the secret value from the same source used to set the child env
and perform replacements on each line before logging so no raw credentials from
stderr.lines are emitted.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f13ecac4-5b04-48b2-829a-ad22351f3234
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
.env.exampleDockerfileMakefileREADME.mdcharts/hub/templates/NOTES.txtcharts/hub/values.yamlcmd/api/app.gocmd/api/app_test.gogo.modinternal/api/handlers/openapi_handler.gointernal/api/handlers/openapi_handler_test.gointernal/config/config.gointernal/config/config_test.goopenapi.yamltests/mcp_smoke_test.go
…uction_ready # Conflicts: # Makefile
BhagyaAmarasinghe
approved these changes
May 5, 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
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.
What does this PR do?
Fixes https://linear.app/formbricks/issue/ENG-759/hub-mcp-server-production-ready
This makes the Hub-side self-hosted MCP path production-ready enough for generated SDK/MCP clients to discover and target a real deployed Hub instance:
/openapi.yamland/openapi.json.PUBLIC_BASE_URLso self-hosted operators can advertise the correct public Hub URL behind ingress, TLS termination, reverse proxies, and path prefixes.PUBLIC_BASE_URLin.env.example, the Helm chart values, and Helm chart notes.openapi.yamlinto the runtime Docker image so the public spec endpoints work in production containers.Remaining ENG-759 completion items outside this Hub PR:
@formbricks/hub-mcppackage changes.hub.formbricks.com.#engineering-chat.hub-typescriptpreview build/lint regression before release.How should this be tested?
make buildmake testsmake lintmake lint-openapiFor
PUBLIC_BASE_URL, start Hub with a production-style value and verify both spec formats advertise it:Expected result: both responses include
https://hub.example.comas the current Hub server URL and/v1/*routes remain API-key protected.Checklist
Required
make buildmake tests(integration tests intests/)make fmtandmake lint; no new warningsgit pull origin mainmigrations/with goose annotations and ranmake migrate-validateAppreciated
make testsor API contract workflow)docs/if changes were necessarymake tests-coveragefor meaningful logic changes