Skip to content

feat(mcp): add a typed posthog telemetry wrapper for the remote server - #6355

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6235
Jul 16, 2026
Merged

feat(mcp): add a typed posthog telemetry wrapper for the remote server#6355
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6235

Conversation

@nghetienhiep

Copy link
Copy Markdown
Contributor

Summary

  • Adds src/mcp/telemetry.ts — a thin, typed wrapper around the PostHog Node SDK, the foundation the rest of the MCP-telemetry work (Spec: usage-telemetry instrumentation strategy for MCP (PostHog) #6228) builds on. It exposes a single entry point, recordMcpToolCall(env, { tool, callerType, ok, durationMs }), and no other module ever constructs a raw PostHog event.
  • The Spec: usage-telemetry instrumentation strategy for MCP (PostHog) #6228 tracked-field allowlist (tool name + caller type + success + coarse duration — and nothing else: no arguments, no source, no wallet/hotkey/trust-score data) is enforced at the type level via McpToolCallEvent; the built PostHog properties are exactly those four fields, asserted in the tests.
  • Safe no-op when unconfigured: with no POSTHOG_API_KEY (every self-hoster who doesn't opt in) it records nothing and behaves byte-identically to before this module existed. It also never throws — a PostHog init/capture failure degrades to recording nothing, so it can never surface an error into the MCP tool caller.
  • Per the issue, this is not wired into the tool-dispatch path yet (that, plus the live-Worker client flush/lifecycle strategy, is the separate instrumentation issue's job).
  • Declares the two opt-in secrets it reads (POSTHOG_API_KEY, POSTHOG_HOST) in src/env.d.ts, mirroring the existing opt-in-secret pattern (e.g. the Orb App credentials), and adds posthog-node to the root package.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changedsrc/mcp/telemetry.ts is at 100% statements/branches/functions/lines (8 tests); src/env.d.ts is Codecov-ignored.
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm audit --audit-level=moderate (0 vulnerabilities)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • test:workers, ui:openapi:check, ui:lint, ui:typecheck, ui:build: no UI, OpenAPI, or Worker-pool surface is touched — the change is a single unimported backend module (src/mcp/telemetry.ts is not yet referenced by any entrypoint), a Codecov-ignored env.d.ts declaration, and a dependency addition.
  • Three pre-existing test/unit/selfhost-*reporting*/grafana suites fail locally with spawnSync sqlite3 ENOENT (the sqlite3 CLI is absent from this sandbox); they are unrelated to this change and pass on CI where the binary is installed.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A: no auth/session/CORS surface changed.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A: no UI change.
  • Visible UI changes include a UI Evidence section. — N/A: no visible UI/frontend/docs change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

Closes #6235

Introduce src/mcp/telemetry.ts, a thin typed wrapper around the PostHog
Node SDK that is the single seam for recording an MCP tool call. The
tracked-field allowlist from the telemetry spec (tool name, caller type,
success, coarse duration -- nothing else) is enforced at the type level,
so no caller outside this module constructs a raw PostHog event.

Telemetry is opt-in: with no POSTHOG_API_KEY configured, recordMcpToolCall
is a safe no-op that records nothing and behaves byte-identically to
before this module existed, and it never throws a PostHog init/capture
failure into the tool caller. Not yet wired into the dispatch path.

Closes JSONbored#6235
@nghetienhiep
nghetienhiep requested a review from JSONbored as a code owner July 16, 2026 03:49
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 03:55:22 UTC

5 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This adds a self-contained, unwired telemetry wrapper (src/mcp/telemetry.ts) that builds a PostHog client per call, sends exactly four allowlisted properties, and never throws — falling back to a no-op when POSTHOG_API_KEY is unset or blank. The type-level allowlist enforcement and the safe-no-op/never-throw guarantees are both verified by direct tests against the mocked SDK, and since the module is explicitly not wired into any dispatch path yet, there's no behavioral risk to existing functionality.

Nits — 5 non-blocking
  • src/mcp/telemetry.ts:39-40 constructs a new PostHog client on every call instead of caching/reusing one; with flushAt:1/flushInterval:0 this is functionally fine but wasteful once this is wired into a hot dispatch path — worth flagging now since the caller-facing API (recordMcpToolCall(env, event)) will be harder to change once call sites exist.
  • No client.shutdown()/flush() is called after capture(); with flushAt:1 the event is sent immediately so this is likely fine, but confirm the PostHog Node SDK doesn't need an explicit shutdown to avoid dangling timers/handles in a short-lived Worker invocation.
  • The PR description says this doesn't upload source/arguments, but there's no runtime validation that `tool` (a plain string) can't be misused by a future caller to smuggle extra data via the tool name itself — likely out of scope here but worth a one-line note in the allowlist comment.
  • Consider caching the PostHog client instance (module-level, keyed by apiKey/host) once this is wired into the dispatch path, to avoid a new HTTP client per tool call under load.
  • Confirm whether `client.shutdown()` is needed for the Cloudflare Worker runtime's request lifecycle before this is wired in — the follow-up issue should address it explicitly.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6235
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 126 registered-repo PR(s), 73 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetienhiep; Gittensor profile; 126 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds the posthog-node dependency and a typed src/mcp/telemetry.ts wrapper exposing recordMcpToolCall with exactly the allowlisted fields, is a verified safe no-op (never throws) when POSTHOG_API_KEY is unset, and is not wired into tool-dispatch, matching all stated requirements. Tests cover both configured and unconfigured paths plus error handling, satisfying the coverage requirement.

Review context
  • Author: nghetienhiep
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 126 PR(s), 0 issue(s).
  • Related work: Titles/paths share 10 meaningful terms. (issue #6235, issue #6236)
  • Related work: Titles/paths share 12 meaningful terms. (issue #6238, issue #6237)
  • Related work: Titles/paths share 8 meaningful terms. (issue #6235, issue #6237)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit d6fa47a into JSONbored:main Jul 16, 2026
13 checks passed
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (c45f629) to head (cf296b5).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6355   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         597      598    +1     
  Lines       47191    47202   +11     
  Branches    15019    15022    +3     
=======================================
+ Hits        45117    45128   +11     
  Misses       1290     1290           
  Partials      784      784           
Flag Coverage Δ
shard-1 44.14% <0.00%> (-0.02%) ⬇️
shard-2 36.39% <0.00%> (-0.01%) ⬇️
shard-3 32.53% <0.00%> (-0.01%) ⬇️
shard-4 34.61% <0.00%> (-0.02%) ⬇️
shard-5 31.58% <100.00%> (+<0.01%) ⬆️
shard-6 44.87% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/mcp/telemetry.ts 100.00% <100.00%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(mcp): add a typed PostHog wrapper module for src/mcp/server.ts (remote)

1 participant