chore(telemetry): remove hardcoded PostHog key default#131
chore(telemetry): remove hardcoded PostHog key default#131willwashburn wants to merge 4 commits into
Conversation
Drop the baked-in `phc_OAqBdey9...` PostHog write key from the MCP client and marketing site. Telemetry now silently no-ops when no key is configured via env / meta tag, so forks and local dev no longer emit events into our production PostHog project. Production builds get the key injected from a `RELAYCAST_POSTHOG_API_KEY` GitHub Actions secret at deploy time (site) or from the user-installed env (npm package). Relaycast half of relay#881 (P0.5). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTelemetry is opt-in via a PostHog API key: backend gating and server wiring skip capture when no key is present, the frontend reads a meta placeholder and disables tracking if invalid/missing, and CI can inject the key into the HTML placeholder at deploy time. ChangesConditional PostHog Telemetry
Sequence DiagramsequenceDiagram
participant GitHub as GitHub Actions
participant HTML as site/index.html
participant Frontend as site/script.js
participant Backend as packages/mcp/src/telemetry.ts
GitHub->>HTML: inject RELAYCAST_POSTHOG_API_KEY into __RELAYCAST_POSTHOG_API_KEY__
Frontend->>HTML: read meta tag 'relaycast-posthog-key'
alt valid key (phc_ prefix)
Frontend->>Backend: createMcpTelemetry with posthogApiKey
Backend->>Backend: telemetryEnabled checks key → true
Backend->>Backend: capture sends events
else no key or invalid
Frontend->>Backend: createMcpTelemetry without key
Backend->>Backend: telemetryEnabled checks key → false
Backend->>Backend: capture is no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| ?? '' | ||
| ); |
There was a problem hiding this comment.
🔴 MCP telemetry silently disabled in Cloudflare Worker production after removing hardcoded key
The removal of DEFAULT_POSTHOG_PUBLIC_KEY breaks MCP telemetry in the Cloudflare Worker context (McpSessionDO). The getPosthogApiKey function falls through process.env.RELAYCAST_POSTHOG_API_KEY → process.env.POSTHOG_API_KEY → options.posthogApiKey → ''. In Cloudflare Workers, secrets set via wrangler secret put are available through the env bindings parameter (as used by the server package at packages/server/src/lib/telemetry.ts:36), not through process.env. Since McpSessionDO (packages/server/src/durable-objects/mcpSession.ts:51-57) calls createRelayMcpServer without passing posthogApiKey, and the MCP server's createRelayMcpServer (packages/mcp/src/server.ts:83-87) doesn't pass it to createMcpTelemetry either, the API key resolves to ''. With the new !apiKey check at packages/mcp/src/telemetry.ts:47, telemetry is disabled. This silently drops all MCP-specific events (relaycast_mcp_server_started, relaycast_mcp_session_authenticated, etc.) in production. Previously, the hardcoded DEFAULT_POSTHOG_PUBLIC_KEY ensured these events were always captured.
Prompt for agents
The core issue is that the MCP package's telemetry uses process.env to read the PostHog API key, but in the Cloudflare Worker runtime (McpSessionDO), secrets are only available through the env bindings parameter, not process.env. The McpSessionDO has access to this.env.POSTHOG_API_KEY, but it never passes it through to createRelayMcpServer or createMcpTelemetry.
To fix this, you need to thread the PostHog API key from the Worker env bindings into the MCP telemetry. One approach:
1. Add a posthogApiKey field to McpServerOptions in packages/mcp/src/server.ts
2. Pass it through when creating telemetry at server.ts:83-87
3. In McpSessionDO.ensureInitialized() (packages/server/src/durable-objects/mcpSession.ts:51), pass posthogApiKey: this.env.POSTHOG_API_KEY to createRelayMcpServer
This mirrors how the server package's own telemetry (packages/server/src/lib/telemetry.ts:36) explicitly reads env.POSTHOG_API_KEY from the Cloudflare bindings rather than relying on process.env.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Preview deployed!
This preview shares the staging database and will be cleaned up when the PR is merged or closed. Run E2E testsnpm run e2e -- https://pr131-api.relaycast.dev --ciOpen observer dashboard |
PostHog `phc_*` project keys are ingestion-only and public by design (same category as Sentry DSN / Stripe publishable key), so the GitHub Actions value is a *variable*, not a secret. Also unifies the name across relay / relaycast / cloud to `POSTHOG_PROJECT_KEY` so one value can be set per repo. Only swaps the `deploy-pages` site-injection step (added in this PR). The pre-existing `deploy-production` step that writes a Cloudflare runtime secret via `wrangler secret put` is left untouched — its GitHub source is a separate value managed independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@copilot please fix issues raised by comments in this PR |
…o MCP telemetry McpSessionDO now passes posthogApiKey: this.env.POSTHOG_API_KEY to createRelayMcpServer, which forwards it to createMcpTelemetry. In Cloudflare Workers, wrangler secrets are only available through the env bindings parameter, not process.env. Without this fix, removing the hardcoded DEFAULT_POSTHOG_PUBLIC_KEY caused all MCP telemetry events (relaycast_mcp_server_started, relaycast_mcp_session_authenticated, etc.) to be silently dropped in production. Agent-Logs-Url: https://github.com/AgentWorkforce/relaycast/sessions/9bb31070-c806-493c-8d06-8eb2a710f826 Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>
Fixed in c4050ff. The review identified that in Cloudflare Workers, The fix threads the key through three layers:
This mirrors how the server package's own telemetry already reads Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…test Round out the previous fix (c4050ff) by also threading POSTHOG_HOST through the same Cloudflare Worker env-bindings path used for POSTHOG_API_KEY, so a custom PostHog host (e.g. EU region) can be configured per environment without relying on process.env in Workers. - McpServerOptions gains an optional `posthogHost` alongside the existing `posthogApiKey`. - McpSessionDO forwards `this.env.POSTHOG_HOST` into createRelayMcpServer. - Adds an explicit regression test that mirrors the CF Worker scenario: process.env.* is unset, options.posthogApiKey is provided, and the capture HTTP call still fires with the expected api_key. This locks in the fix Devin AI flagged on PR #131. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks @devin-ai-integration[bot] — the regression you flagged is real and is now fixed. Resolution
Verification:
Heads-up on the other findings Your comment mentions "4 additional findings in Devin Review" living on the external dashboard at |
Summary
Relaycast half of AgentWorkforce/relay#881 (P0.5). A parallel PR in the
relayrepo handles the relay-side changes.Drops the baked-in
phc_OAqBdey9...PostHog write key from this repo so that:Why a variable, not a secret: PostHog
phc_*project keys are ingestion-only and public by design — same category as a Sentry DSN or Stripe publishable key. The structural problem is fork pollution, and that's already solved by (a) the new no-op-when-missing behavior and (b) the fact that GitHub gates bothvarsandsecretsfrom fork PRs. Calling it a secret was misleading.What changed
packages/mcp/src/telemetry.tsDEFAULT_POSTHOG_PUBLIC_KEYconstant.getPosthogApiKey()now falls back to''(empty string) when no env / option is provided.telemetryEnabled()now takes the resolved API key and returnsfalsewhen it's empty — same code path as the existingDO_NOT_TRACK/RELAYCAST_TELEMETRY_DISABLEDopt-outs, socapture()silently no-ops with no HTTP calls.RELAYCAST_POSTHOG_API_KEY>POSTHOG_API_KEY>options.posthogApiKey.packages/mcp/src/__tests__/telemetry.test.tsposthogApiKey: 'phc_example'(previously relied on the hardcoded default).silently no-ops when no PostHog API key is configured.site/index.htmlphc_OAqBdey9...meta value with a__RELAYCAST_POSTHOG_API_KEY__placeholder that CI substitutes at deploy time.site/script.jsPOSTHOG_PUBLIC_KEYconstant fallback.startsWith('phc_')guard so the unsubstituted__RELAYCAST_POSTHOG_API_KEY__placeholder (or any other non-phc value) is treated as "no key configured" and the analytics layer returns a no-opcapture. Forks deploying their own copy of the static site, and local previews viafile://, both get no-op behavior..github/workflows/deploy.ymlInject PostHog API key into marketing sitestep beforepages deploy site/. Uses a small Node script to do a literal__RELAYCAST_POSTHOG_API_KEY__-> value substitution insite/index.html. The source isvars.POSTHOG_PROJECT_KEY(a repository variable, not a secret — see "why" above). If the variable is unset, logs a warning and ships the site with telemetry disabled (no failure).deploy-productionjob already pipessecrets.POSTHOG_API_KEYfrom a GitHub secret into the Cloudflare Worker viawrangler secret put— left as-is. The Cloudflare-side is still a runtime secret (that's a Cloudflare concept), and the GitHub source for that step is a separate value managed independently from this PR.Workflows intentionally NOT modified
publish-npm.yml— the MCP package readsRELAYCAST_POSTHOG_API_KEYfrom the user's runtime env at install time; nothing is baked into the npm artifact.publish-rust.yml,local-binary-release.yml— no PostHog integration in those artifacts.ci.ymland thecijob indeploy.yml— left without the key so test events from CI don't pollute the prod project.DO_NOT_TRACK=1was already set on test runs there.Action required after merge
Add
POSTHOG_PROJECT_KEYas a repository Variable (Settings → Secrets and variables → Actions → Variables tab, NOT Secrets) before the next release. Thedeploy-pagesjob injects this value into the marketing site at deploy time. Without it, the site ships cleanly but with telemetry disabled.The Cloudflare Worker still receives its PostHog value as a runtime secret via
wrangler secret put POSTHOG_API_KEY(that's a Cloudflare-side concept, not a GitHub-side one). The existingsecrets.POSTHOG_API_KEYGitHub Actions secret feeding that step is unchanged by this PR.Users running the MCP locally won't have a key set by default and will silently no-op. If we want to opt them in via documentation, that's a follow-up.
Notes / judgment calls
site/script.jsreject any meta value that doesn't start withphc_. This handles the__RELAYCAST_POSTHOG_API_KEY__placeholder cleanly and also defensively rejects garbage values. Felt cleaner than avalue === '__RELAYCAST_POSTHOG_API_KEY__'string compare.sed -i) to avoid GNU vs. BSDseddifferences and any regex-special-char surprises in the key. It also validates the placeholder is present and exits non-zero if it isn't.docs/or marketing-copy mentions of the key needed updating (verified bygrep -r phc_OAqBdey9 .).Test plan
npm run build— succeeds (turbo, 9 tasks).DO_NOT_TRACK=1 npm test— 223/223 MCP tests pass; 437/437 server tests pass; full suite green.npm run lint— 0 errors (64 pre-existing warnings unrelated to this change).grep -r 'phc_OAqBdey9'returns 0 hits across the repo.<meta name="relaycast-posthog-key">contains the real key in the deployedsite/index.html.npx @relaycast/mcplocally with no env, confirm no PostHog requests in network logs.