Skip to content

feat(mcp): add a typed PostHog wrapper module for packages/loopover-mcp - #6368

Merged
JSONbored merged 3 commits into
JSONbored:mainfrom
claytonlin1110:feat/mcp-local-telemetry-wrapper-6236
Jul 16, 2026
Merged

feat(mcp): add a typed PostHog wrapper module for packages/loopover-mcp#6368
JSONbored merged 3 commits into
JSONbored:mainfrom
claytonlin1110:feat/mcp-local-telemetry-wrapper-6236

Conversation

@claytonlin1110

Copy link
Copy Markdown
Contributor

Summary

  • Part of Spec: usage-telemetry instrumentation strategy for MCP (PostHog) #6228. Adds packages/loopover-mcp/lib/telemetry.js, a typed recordMcpToolCall({ tool, callerType, ok, durationMs }) wrapper for the local stdio MCP server, mirroring the already-merged remote wrapper's shape (src/mcp/telemetry.ts, feat(mcp): add a typed PostHog wrapper module for src/mcp/server.ts (remote) #6235) and PostHog event schema so both servers report consistent data.
  • Unlike the remote wrapper (gated on POSTHOG_API_KEY presence alone), this one is gated on an explicit, persisted opt-in flagoptions.telemetryEnabled must be exactly true — before it even looks at an API key. This matches Spec: usage-telemetry instrumentation strategy for MCP (PostHog) #6228's stated privacy decision: a locally-run CLI needs opt-in, not opt-out, since it executes on a user's own machine. The already-merged loopover-mcp telemetry enable|disable command (feat(mcp): add telemetry enable/disable CLI command with persisted opt-in #6348) is the mechanism that persists this flag; this module stays a pure lib/ helper and never reads CLI config itself — the caller resolves the flag and passes it in.
  • The PostHog API key/host are read from LOOPOVER_MCP_POSTHOG_API_KEY / LOOPOVER_MCP_POSTHOG_HOST env vars, mirroring the remote wrapper's env-based configuration approach. No key configured ⇒ safe no-op, same as the remote side.
  • Never throws: a PostHog init/capture failure degrades to recording nothing, so it can never affect the CLI's actual command behavior. This issue only adds the wrapper module itself — wiring it into the tool-dispatch path is the separate, still-open feat(mcp): instrument the local MCP tool-dispatch chokepoint with PostHog events (opt-in) #6238.
  • Added posthog-node as a dependency of packages/loopover-mcp (a separately-published npm package, so it needs its own explicit dependency declaration) and wired the new lib/telemetry.js into the package's existing node --check-per-file build script.

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 changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This local Windows dev environment cannot run wrangler (cf-typegen:check fails with a pre-existing, change-unrelated spawnSync wrangler ENOENT), which blocks the chained npm run test:ci from reaching later steps locally. Each step was therefore run individually: git diff --check, actionlint, db:migrations:check, db:schema-drift:check, selfhost:env-reference:check, miner:env-reference:check, selfhost:validate-observability, build:mcp, engine build, typecheck, test:engine-parity, test:live-gate-parity, and test:driver-parity all pass clean on this branch. test:mcp-pack also fails locally with a pre-existing, change-unrelated spawnSync npm ERR_INVALID_ARG_TYPE (confirmed identical on a clean upstream/main checkout via git stash) — the same class of Windows-only spawnSync cross-platform issue as cf-typegen. The new module and its 11 tests (test/unit/mcp-local-telemetry.test.ts) pass cleanly and cover both the opted-in and default-off no-op paths, the missing/blank-API-key no-op paths, host override/trim/default-fallback, and both PostHog init- and capture-failure never-throw paths — mirroring test/unit/mcp-telemetry.test.ts's coverage of the sibling remote wrapper. The broader mcp-cli-* sweep passes cleanly aside from 3 pre-existing, change-unrelated Windows-only symlink-permission failures (EPERM: operation not permitted, symlink).

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.
  • 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.
  • Visible UI changes include a UI Evidence section below — N/A, no UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

N/A — backend-only observability change, no visible UI change.

Notes

Mirrors the remote wrapper's recordMcpToolCall shape (JSONbored#6235) for the
local stdio server, but gated on an explicit, persisted opt-in flag
(default OFF) rather than mere API-key presence, since a locally-run
CLI is a different trust posture than the hosted remote server. The
module stays a pure lib/ helper -- the caller resolves the opt-in
flag from CLI config and passes it in -- and never throws.

Closes JSONbored#6236
@superagent-security

Copy link
Copy Markdown
Contributor

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

check-mcp-package.mjs's dry-run gate rejects any file the workspace's
"files" glob would ship that isn't on this shared allowlist -- the new
lib/telemetry.js wrapper (JSONbored#6236) needed adding here too.
@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
@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 (b7e1bec) to head (d280ce4).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6368   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         599      599           
  Lines       47221    47221           
  Branches    15024    15024           
=======================================
  Hits        45147    45147           
  Misses       1290     1290           
  Partials      784      784           
Flag Coverage Δ
shard-1 44.07% <ø> (-0.11%) ⬇️
shard-2 36.60% <ø> (+0.20%) ⬆️
shard-3 32.52% <ø> (ø)
shard-4 34.61% <ø> (-0.01%) ⬇️
shard-5 31.60% <ø> (ø)
shard-6 44.88% <ø> (ø)

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

@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-16 05:40:24 UTC

6 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): .github/workflows/ci.yml (matched .github/workflows/**).

Review summary
This adds a self-contained, never-throw telemetry wrapper for the local MCP CLI, gated on an explicit `telemetryEnabled === true` flag plus a configured API key, mirroring the already-merged remote wrapper's event shape. The module is not yet wired into the tool-dispatch path (explicitly deferred to #6238), and the diff also bundles an unrelated CI cache-hit fix for fumadocs codegen — both are logically sound but the CI cache change is scope creep for a 'feat(mcp)' PR titled around the telemetry wrapper. Tests are thorough and exercise the real exported function against a mocked posthog-node module, covering both no-op paths and the never-throw guarantees.

Nits — 6 non-blocking
  • The CI workflow change in .github/workflows/ci.yml (regenerating fumadocs codegen on cache hit) is unrelated to the stated MCP telemetry feature and should probably be its own PR per this repo's 'focused, no unrelated changes' scope rule.
  • packages/loopover-mcp/lib/telemetry.js:16 hardcodes the PostHog US-cloud host as a fallback default, which is reasonable as a default but worth confirming matches the remote wrapper's exact same default so the two stay in lockstep.
  • The module is unused/unwired in this PR (by design, deferred to feat(mcp): instrument the local MCP tool-dispatch chokepoint with PostHog events (opt-in) #6238) — worth noting in the PR body that `npm run build`'s node --check only validates syntax, not that the export is consumed anywhere yet.
  • CI shows validate/validate-code/validate-tests-merge and several validate-tests shards as FAILED on this commit, which the review should have visibility into even though the instructions say not to predict CI — worth the contributor double-checking why before merge.
  • Split the .github/workflows/ci.yml fumadocs-codegen fix into a separate PR to keep this one scoped to the telemetry wrapper per the repo's own PR checklist.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ⚠️ Gate result — Not blocking (Advisory; not blocking this PR.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6236
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: 384 registered-repo PR(s), 256 merged, 109 issue(s).
Contributor context ✅ Confirmed Gittensor contributor claytonlin1110; Gittensor profile; 384 PR(s), 109 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 3 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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://loopover.ai/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

apps/loopover-ui/.source (the collections/* alias apps/loopover-ui/
tsconfig.json depends on) is produced by npm ci's own postinstall,
which the validate-code job's node_modules cache hit skips entirely --
.source/ lives outside node_modules, so it never gets generated on a
cache-hit run and ui:typecheck fails with "Cannot find module
'collections/browser'/'collections/server'". Confirmed this reproduces
identically on main regardless of PR contents. Regenerating it
explicitly here is cheap and a harmless no-op re-run on cache miss.
@claytonlin1110
claytonlin1110 marked this pull request as ready for review July 16, 2026 05:35
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 16, 2026
@JSONbored
JSONbored merged commit 7152db6 into JSONbored:main Jul 16, 2026
16 checks passed
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(mcp): add a typed PostHog wrapper module for packages/loopover-mcp (local, opt-in)

2 participants