From 85e440a641edff682b6cb444b10476fb064bcb21 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Thu, 6 Aug 2026 17:32:36 -0700 Subject: [PATCH 1/3] Add anonymous usage telemetry to the MCP server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP server reported nothing about its own usage, so the only signal for "is anyone running this?" was npm download counts — which conflate CI, mirrors and repeat installs, and say nothing about whether the server ever started or whether the devtools transport actually connected. Instrument it with @posthog/mcp, which reports server start, tool calls, latency and errors, plus which MCP client is driving it. Privacy is enforced by an allowlist rather than a denylist. The SDK captures tool arguments and responses unconditionally — there is no option to disable it, and its built-in sanitizer only matches sensitive key *names*, so a field named `flowContent` would be transmitted verbatim. Devtools tool arguments carry player/plugin ids and invoke_action payloads, i.e. potentially customer flow data. Every event is therefore filtered through an explicit set of known-safe property names, installed as a posthog-node client-level `before_send`: a chokepoint the SDK cannot route around, since we own the client and it does not. A denylist would silently start leaking the first time a 0.x release added a new payload property. The SDK's `context` option is disabled. It defaults to ON and would inject a required `context` parameter into every tool schema, mutating our public API to ask the calling agent to narrate user intent. Identity is a random UUID in ~/.player-ui-devtools/install.json — deliberately not beside the flipper refcount in the OS temp dir, which is periodically reaped and would turn "unique installs" into "unique boots". When the home directory is unwritable the id is null and telemetry is skipped, rather than minting a per-run id that would inflate install counts. Telemetry is on by default and opts out via PLAYER_DEVTOOLS_TELEMETRY_DISABLED or the cross-vendor DO_NOT_TRACK convention. The shipped project key is a placeholder, so this is inert until one is provisioned. @posthog/mcp requires Node ^20.20.0 || >=22.22.0, which excludes the previous 22.15.1 toolchain pin; rules_nodejs is bumped to 6.7.5 as 6.4.0 does not know any Node newer than 22.15.1. Also replaces the hardcoded, stale server version "0.0.1" with the stamped __VERSION__ global, and flushes buffered events in stop() before bin/run calls process.exit(0). --- MODULE.bazel | 7 +- MODULE.bazel.lock | 140 ++---------------- devtools/mcp/BUILD | 3 + devtools/mcp/README.md | 31 +++- devtools/mcp/package.json | 3 + devtools/mcp/src/__tests__/server.test.ts | 78 ++++++++++ devtools/mcp/src/server.ts | 22 ++- .../src/telemetry/__tests__/config.test.ts | 57 +++++++ .../mcp/src/telemetry/__tests__/index.test.ts | 113 ++++++++++++++ .../src/telemetry/__tests__/installId.test.ts | 83 +++++++++++ .../src/telemetry/__tests__/redaction.test.ts | 87 +++++++++++ devtools/mcp/src/telemetry/config.ts | 51 +++++++ devtools/mcp/src/telemetry/index.ts | 85 +++++++++++ devtools/mcp/src/telemetry/installId.ts | 79 ++++++++++ devtools/mcp/src/telemetry/redaction.ts | 72 +++++++++ package.json | 6 +- pnpm-lock.yaml | 50 +++++++ 17 files changed, 832 insertions(+), 135 deletions(-) create mode 100644 devtools/mcp/src/__tests__/server.test.ts create mode 100644 devtools/mcp/src/telemetry/__tests__/config.test.ts create mode 100644 devtools/mcp/src/telemetry/__tests__/index.test.ts create mode 100644 devtools/mcp/src/telemetry/__tests__/installId.test.ts create mode 100644 devtools/mcp/src/telemetry/__tests__/redaction.test.ts create mode 100644 devtools/mcp/src/telemetry/config.ts create mode 100644 devtools/mcp/src/telemetry/index.ts create mode 100644 devtools/mcp/src/telemetry/installId.ts create mode 100644 devtools/mcp/src/telemetry/redaction.ts diff --git a/MODULE.bazel b/MODULE.bazel index 51489dd..b6b775d 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -18,10 +18,13 @@ bazel_dep(name = "rules_pkg", version = "1.1.0") bazel_dep(name = "aspect_rules_ts", version = "3.6.3") ####### Node.js version ######### -bazel_dep(name = "rules_nodejs", version = "6.4.0") +bazel_dep(name = "rules_nodejs", version = "6.7.5") node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node") -node.toolchain(node_version = "22.15.1") + +# @posthog/mcp requires "^20.20.0 || >=22.22.0"; 22.22.0 is the first satisfying +# release, and it is only known to rules_nodejs >= 6.7.x. +node.toolchain(node_version = "22.22.0") ################################# npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm") diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 042d14c..255d441 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -52,10 +52,13 @@ "https://bcr.bazel.build/modules/bazel_features/1.28.0/MODULE.bazel": "4b4200e6cbf8fa335b2c3f43e1d6ef3e240319c33d43d60cc0fbd4b87ece299d", "https://bcr.bazel.build/modules/bazel_features/1.3.0/MODULE.bazel": "cdcafe83ec318cda34e02948e81d790aab8df7a929cec6f6969f13a489ccecd9", "https://bcr.bazel.build/modules/bazel_features/1.30.0/MODULE.bazel": "a14b62d05969a293b80257e72e597c2da7f717e1e69fa8b339703ed6731bec87", - "https://bcr.bazel.build/modules/bazel_features/1.30.0/source.json": "b07e17f067fe4f69f90b03b36ef1e08fe0d1f3cac254c1241a1818773e3423bc", + "https://bcr.bazel.build/modules/bazel_features/1.39.0/MODULE.bazel": "28739425c1fc283c91931619749c832b555e60bcd1010b40d8441ce0a5cf726d", + "https://bcr.bazel.build/modules/bazel_features/1.39.0/source.json": "f63cbeb4c602098484d57001e5a07d31cb02bbccde9b5e2c9bf0b29d05283e93", "https://bcr.bazel.build/modules/bazel_features/1.4.1/MODULE.bazel": "e45b6bb2350aff3e442ae1111c555e27eac1d915e77775f6fdc4b351b758b5d7", "https://bcr.bazel.build/modules/bazel_features/1.9.0/MODULE.bazel": "885151d58d90d8d9c811eb75e3288c11f850e1d6b481a8c9f766adee4712358b", "https://bcr.bazel.build/modules/bazel_features/1.9.1/MODULE.bazel": "8f679097876a9b609ad1f60249c49d68bfab783dd9be012faf9d82547b14815a", + "https://bcr.bazel.build/modules/bazel_lib/3.0.0/MODULE.bazel": "22b70b80ac89ad3f3772526cd9feee2fa412c2b01933fea7ed13238a448d370d", + "https://bcr.bazel.build/modules/bazel_lib/3.0.0/source.json": "895f21909c6fba01d7c17914bb6c8e135982275a1b18cdaa4e62272217ef1751", "https://bcr.bazel.build/modules/bazel_skylib/1.0.3/MODULE.bazel": "bcb0fd896384802d1ad283b4e4eb4d718eebd8cb820b0a2c3a347fb971afd9d8", "https://bcr.bazel.build/modules/bazel_skylib/1.1.1/MODULE.bazel": "1add3e7d93ff2e6998f9e118022c84d163917d912f5afafb3058e3d2f1545b5e", "https://bcr.bazel.build/modules/bazel_skylib/1.2.0/MODULE.bazel": "44fe84260e454ed94ad326352a698422dbe372b21a1ac9f3eab76eb531223686", @@ -110,13 +113,14 @@ "https://bcr.bazel.build/modules/pathkit/1.0.1/source.json": "3215e6b4b08f96f34024eaf186d247744ca255925d7ee3f50cf94f7cf885696b", "https://bcr.bazel.build/modules/platforms/0.0.10/MODULE.bazel": "8cb8efaf200bdeb2150d93e162c40f388529a25852b332cec879373771e48ed5", "https://bcr.bazel.build/modules/platforms/0.0.11/MODULE.bazel": "0daefc49732e227caa8bfa834d65dc52e8cc18a2faf80df25e8caea151a9413f", - "https://bcr.bazel.build/modules/platforms/0.0.11/source.json": "f7e188b79ebedebfe75e9e1d098b8845226c7992b307e28e1496f23112e8fc29", "https://bcr.bazel.build/modules/platforms/0.0.4/MODULE.bazel": "9b328e31ee156f53f3c416a64f8491f7eb731742655a47c9eec4703a71644aee", "https://bcr.bazel.build/modules/platforms/0.0.5/MODULE.bazel": "5733b54ea419d5eaf7997054bb55f6a1d0b5ff8aedf0176fef9eea44f3acda37", "https://bcr.bazel.build/modules/platforms/0.0.6/MODULE.bazel": "ad6eeef431dc52aefd2d77ed20a4b353f8ebf0f4ecdd26a807d2da5aa8cd0615", "https://bcr.bazel.build/modules/platforms/0.0.7/MODULE.bazel": "72fd4a0ede9ee5c021f6a8dd92b503e089f46c227ba2813ff183b71616034814", "https://bcr.bazel.build/modules/platforms/0.0.8/MODULE.bazel": "9f142c03e348f6d263719f5074b21ef3adf0b139ee4c5133e2aa35664da9eb2d", "https://bcr.bazel.build/modules/platforms/0.0.9/MODULE.bazel": "4a87a60c927b56ddd67db50c89acaa62f4ce2a1d2149ccb63ffd871d5ce29ebc", + "https://bcr.bazel.build/modules/platforms/1.0.0/MODULE.bazel": "f05feb42b48f1b3c225e4ccf351f367be0371411a803198ec34a389fb22aa580", + "https://bcr.bazel.build/modules/platforms/1.0.0/source.json": "f4ff1fd412e0246fd38c82328eb209130ead81d62dcd5a9e40910f867f733d96", "https://bcr.bazel.build/modules/protobuf/21.7/MODULE.bazel": "a5a29bb89544f9b97edce05642fac225a808b5b7be74038ea3640fae2f8e66a7", "https://bcr.bazel.build/modules/protobuf/23.1/MODULE.bazel": "88b393b3eb4101d18129e5db51847cd40a5517a53e81216144a8c32dfeeca52a", "https://bcr.bazel.build/modules/protobuf/24.4/MODULE.bazel": "7bc7ce5f2abf36b3b7b7c8218d3acdebb9426aeb35c2257c96445756f970eb12", @@ -155,7 +159,8 @@ "https://bcr.bazel.build/modules/rules_cc/0.0.9/MODULE.bazel": "836e76439f354b89afe6a911a7adf59a6b2518fafb174483ad78a2a2fde7b1c5", "https://bcr.bazel.build/modules/rules_cc/0.1.1/MODULE.bazel": "2f0222a6f229f0bf44cd711dc13c858dad98c62d52bd51d8fc3a764a83125513", "https://bcr.bazel.build/modules/rules_cc/0.1.2/MODULE.bazel": "557ddc3a96858ec0d465a87c0a931054d7dcfd6583af2c7ed3baf494407fd8d0", - "https://bcr.bazel.build/modules/rules_cc/0.1.2/source.json": "53fcb09b5816c83ca60d9d7493faf3bfaf410dfc2f15deb52d6ddd146b8d43f0", + "https://bcr.bazel.build/modules/rules_cc/0.2.16/MODULE.bazel": "9242fa89f950c6ef7702801ab53922e99c69b02310c39fb6e62b2bd30df2a1d4", + "https://bcr.bazel.build/modules/rules_cc/0.2.16/source.json": "d03d5cde49376d87e14ec14b666c56075e5e3926930327fd5d0484a1ff2ac1cc", "https://bcr.bazel.build/modules/rules_foreign_cc/0.9.0/MODULE.bazel": "c9e8c682bf75b0e7c704166d79b599f93b72cfca5ad7477df596947891feeef6", "https://bcr.bazel.build/modules/rules_fuzzing/0.5.2/MODULE.bazel": "40c97d1144356f52905566c55811f13b299453a14ac7769dfba2ac38192337a8", "https://bcr.bazel.build/modules/rules_fuzzing/0.5.2/source.json": "c8b1e2c717646f1702290959a3302a178fb639d987ab61d548105019f11e527e", @@ -192,8 +197,8 @@ "https://bcr.bazel.build/modules/rules_multitool/1.0.0/source.json": "5b92dc9b267b024bb2174bdf5d8e32821abb53fd84545e00699c4e35826a1c78", "https://bcr.bazel.build/modules/rules_nodejs/6.2.0/MODULE.bazel": "ec27907f55eb34705adb4e8257952162a2d4c3ed0f0b3b4c3c1aad1fac7be35e", "https://bcr.bazel.build/modules/rules_nodejs/6.3.0/MODULE.bazel": "45345e4aba35dd6e4701c1eebf5a4e67af4ed708def9ebcdc6027585b34ee52d", - "https://bcr.bazel.build/modules/rules_nodejs/6.4.0/MODULE.bazel": "ce1fe1795222521d4b4494df58583a06f3bd6dd5d0d9409ea481fcab0e86f852", - "https://bcr.bazel.build/modules/rules_nodejs/6.4.0/source.json": "3e3e8ad75b171b0b67c84fbe624c6d5b0a87b11f0bda17b4050eca95297ae040", + "https://bcr.bazel.build/modules/rules_nodejs/6.7.5/MODULE.bazel": "97e6794043821d23c013baa4a50fd1c599f2e6ae92b06e2c5f1cd7074fd83e7c", + "https://bcr.bazel.build/modules/rules_nodejs/6.7.5/source.json": "d60ee5a76258b1c8f99545ed24172b44d43ba64ca1a2dfc04371ef203df19fdf", "https://bcr.bazel.build/modules/rules_pkg/0.7.0/MODULE.bazel": "df99f03fc7934a4737122518bb87e667e62d780b610910f0447665a7e2be62dc", "https://bcr.bazel.build/modules/rules_pkg/1.0.1/MODULE.bazel": "5b1df97dbc29623bccdf2b0dcd0f5cb08e2f2c9050aab1092fd39a41e82686ff", "https://bcr.bazel.build/modules/rules_pkg/1.1.0/MODULE.bazel": "9db8031e71b6ef32d1846106e10dd0ee2deac042bd9a2de22b4761b0c3036453", @@ -279,7 +284,7 @@ "moduleExtensions": { "@@apple_support+//crosstool:setup.bzl%apple_cc_configure_extension": { "general": { - "bzlTransitiveDigest": "gv4nokEMGNye4Jvoh7Tw0Lzs63zfklj+n4t0UegI7Ms=", + "bzlTransitiveDigest": "3YZIdRYy1ZawZ4nWufHge2uOiqnpOCqqD+zKcdYFqLk=", "usagesDigest": "kbKsOhDawUJi1ZDYHslECzs1HVGdq2xZPmUZdf9ALqg=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -443,7 +448,7 @@ "bzlTransitiveDigest": "aVqwKoRPrSXO367SJABlye04kmpR/9VM2xiXB3nh3Ls=", "usagesDigest": "qH5h0y49b/BYrI5SRoLkSpQctbXmqG8KSHgoA8eopCE=", "recordedFileInputs": { - "@@//package.json": "5360a8a63e9250fdcbc33787bb40ed19f7cd3051410fd59637863d634c91f628" + "@@//package.json": "b6aad65b889b5a2400249868ce69c780c98f4d2d7ede27b8fefff320a909bc42" }, "recordedDirentsInputs": {}, "envVariables": {}, @@ -973,127 +978,6 @@ ] } }, - "@@rules_nodejs+//nodejs:extensions.bzl%node": { - "general": { - "bzlTransitiveDigest": "MLpfA+65GINLc2EHubnI97nNK9NXPVkAUVpbgIrPV/M=", - "usagesDigest": "gZKzN3JCTYjVJHccgM22QsagwtDXd7PCSZXav4Und8M=", - "recordedFileInputs": {}, - "recordedDirentsInputs": {}, - "envVariables": {}, - "generatedRepoSpecs": { - "nodejs_linux_amd64": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "linux_amd64" - } - }, - "nodejs_linux_arm64": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "linux_arm64" - } - }, - "nodejs_linux_s390x": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "linux_s390x" - } - }, - "nodejs_linux_ppc64le": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "linux_ppc64le" - } - }, - "nodejs_darwin_amd64": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "darwin_amd64" - } - }, - "nodejs_darwin_arm64": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "darwin_arm64" - } - }, - "nodejs_windows_amd64": { - "repoRuleId": "@@rules_nodejs+//nodejs:repositories.bzl%_nodejs_repositories", - "attributes": { - "node_download_auth": {}, - "node_repositories": {}, - "node_urls": [ - "https://nodejs.org/dist/v{version}/{filename}" - ], - "node_version": "22.15.1", - "include_headers": false, - "platform": "windows_amd64" - } - }, - "nodejs": { - "repoRuleId": "@@rules_nodejs+//nodejs/private:nodejs_repo_host_os_alias.bzl%nodejs_repo_host_os_alias", - "attributes": { - "user_node_repository_name": "nodejs" - } - }, - "nodejs_host": { - "repoRuleId": "@@rules_nodejs+//nodejs/private:nodejs_repo_host_os_alias.bzl%nodejs_repo_host_os_alias", - "attributes": { - "user_node_repository_name": "nodejs" - } - }, - "nodejs_toolchains": { - "repoRuleId": "@@rules_nodejs+//nodejs/private:nodejs_toolchains_repo.bzl%nodejs_toolchains_repo", - "attributes": { - "user_node_repository_name": "nodejs" - } - } - }, - "recordedRepoMappingEntries": [] - } - }, "@@rules_player+//shell:defs.bzl%sh_configure": { "general": { "bzlTransitiveDigest": "hHBMsvBPNv+aexlCP8obaLYRtsEzjIRKeVm1n2HpZ5s=", diff --git a/devtools/mcp/BUILD b/devtools/mcp/BUILD index ff22e07..7349fb4 100644 --- a/devtools/mcp/BUILD +++ b/devtools/mcp/BUILD @@ -16,6 +16,9 @@ deps = [ ":node_modules/@player-devtools/messenger", ":node_modules/@player-devtools/types", "//:node_modules/@modelcontextprotocol/sdk", + "//:node_modules/@posthog/mcp", + "//:node_modules/@types/node", + "//:node_modules/posthog-node", "//:node_modules/zod", ] diff --git a/devtools/mcp/README.md b/devtools/mcp/README.md index 1d01287..5216133 100644 --- a/devtools/mcp/README.md +++ b/devtools/mcp/README.md @@ -26,8 +26,8 @@ The package ships a CLI, `player-devtools-mcp`, which is what an MCP client runs ### Register with Claude Code -Add it as an MCP server with `claude mcp add` — no env vars or tokens are -required: +Add it as an MCP server with `claude mcp add` — no tokens are required (see +[Telemetry](#telemetry) for the optional opt-out variables): ```bash claude mcp add player-devtools -- npx -y @player-devtools/mcp@latest @@ -135,4 +135,31 @@ just mcp-inspect # open the MCP inspector against the server Register it with an MCP client (e.g. Claude) by pointing the client at the `player-devtools-mcp` command over stdio. +## Telemetry + +The server reports anonymous usage analytics so we can tell how widely it's used +and whether it's working in the field. It is **on by default** and sends: + +| | | +| --- | --- | +| Identity | A random UUID generated on first run and stored at `~/.player-ui-devtools/install.json`. It is not derived from anything about you or your machine — delete the file and a new one is generated. | +| Events | Server start, tool call, tools list, and errors. | +| Properties | Tool **name**, call duration, whether the call errored, the MCP client name/version (e.g. which editor), the devtools version, OS platform, Node major version, and whether the Flipper transport connected. | + +**Tool arguments and tool responses are never transmitted.** Those can contain +Player flow content, so every outgoing event is filtered through an allowlist of +known-safe property names — anything not explicitly listed is dropped before the +event leaves the process. + +To opt out, set either variable to any value: + +```bash +export PLAYER_DEVTOOLS_TELEMETRY_DISABLED=1 +# or the cross-vendor convention, which we also honor +export DO_NOT_TRACK=1 +``` + +`DO_NOT_TRACK=0` and `DO_NOT_TRACK=false` are treated as "tracking is fine", not +as an opt-out. + [browser extension]: https://github.com/player-ui/browser-devtools diff --git a/devtools/mcp/package.json b/devtools/mcp/package.json index 5fdf729..5319aca 100644 --- a/devtools/mcp/package.json +++ b/devtools/mcp/package.json @@ -9,6 +9,9 @@ "dist", "bin" ], + "engines": { + "node": "^20.20.0 || >=22.22.0" + }, "dependencies": { "@player-devtools/client": "workspace:*", "@player-devtools/client-flipper": "workspace:*", diff --git a/devtools/mcp/src/__tests__/server.test.ts b/devtools/mcp/src/__tests__/server.test.ts new file mode 100644 index 0000000..10cab34 --- /dev/null +++ b/devtools/mcp/src/__tests__/server.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { Transport } from "@player-devtools/types"; + +const shutdown = vi.hoisted(() => vi.fn()); +const createAnalytics = vi.hoisted(() => vi.fn()); + +vi.mock("../telemetry", () => ({ + createAnalytics, + MCP_VERSION: "test-version", +})); + +// The stdio transport would otherwise take over the real process stdout. +vi.mock("@modelcontextprotocol/sdk/server/stdio.js", () => ({ + StdioServerTransport: class { + async start(): Promise {} + async send(): Promise {} + async close(): Promise {} + }, +})); + +const { MCPServer } = await import("../server"); + +/** A transport whose connect outcome the test controls. */ +function fakeTransport(connect: () => Promise): Transport { + return { + connect, + close: vi.fn(async () => {}), + sendMessage: vi.fn(async () => {}), + addListener: vi.fn(), + removeListener: vi.fn(), + } as unknown as Transport; +} + +describe("MCPServer", () => { + beforeEach(() => { + vi.clearAllMocks(); + createAnalytics.mockReturnValue({ shutdown }); + }); + + it("starts even when the devtools transport fails to connect", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const server = new MCPServer( + fakeTransport(async () => { + throw new Error("no flipper"); + }), + ); + + await expect(server.start()).resolves.toBeUndefined(); + }); + + it("reports the transport as connected only after a successful connect", async () => { + const server = new MCPServer(fakeTransport(async () => {})); + const { isTransportConnected } = createAnalytics.mock.calls[0]?.[1] ?? {}; + + expect(isTransportConnected()).toBe(false); + await server.start(); + expect(isTransportConnected()).toBe(true); + }); + + it("flushes buffered telemetry before tearing down", async () => { + const transport = fakeTransport(async () => {}); + const server = new MCPServer(transport); + + await server.stop(); + + expect(shutdown).toHaveBeenCalled(); + expect(transport.close).toHaveBeenCalled(); + }); + + it("still shuts down cleanly when the telemetry flush fails", async () => { + shutdown.mockRejectedValueOnce(new Error("network down")); + const transport = fakeTransport(async () => {}); + const server = new MCPServer(transport); + + await expect(server.stop()).resolves.toBeUndefined(); + expect(transport.close).toHaveBeenCalled(); + }); +}); diff --git a/devtools/mcp/src/server.ts b/devtools/mcp/src/server.ts index 1782183..18093f3 100644 --- a/devtools/mcp/src/server.ts +++ b/devtools/mcp/src/server.ts @@ -6,16 +6,27 @@ import type { Transport } from "@player-devtools/types"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import type { PostHog } from "posthog-node"; import { TOOL_DEFS, type ToolDef } from "./tools"; +import { createAnalytics, MCP_VERSION } from "./telemetry"; export class MCPServer { private client: ExtensionClient; private server: McpServer; + private analytics: PostHog | null; + private transportConnected = false; constructor(private transport: Transport) { this.client = createExtensionClient(transport); - this.server = new McpServer({ name: "player-devtools", version: "0.0.1" }); + this.server = new McpServer({ + name: "player-devtools", + version: MCP_VERSION, + }); + // Instrument before registering tools so every handler is wrapped. + this.analytics = createAnalytics(this.server, { + isTransportConnected: () => this.transportConnected, + }); this.registerTools(); } @@ -44,6 +55,7 @@ export class MCPServer { await this.server.connect(stdioTransport); try { await this.transport.connect(); + this.transportConnected = true; } catch (err) { console.warn( "[MCPServer] Transport connect failed (will operate in disconnected mode):", @@ -53,6 +65,14 @@ export class MCPServer { } async stop(): Promise { + // Flush buffered events first: bin/run calls process.exit(0) as soon as + // this resolves, which would otherwise drop them. Never fail shutdown on a + // telemetry error. + try { + await this.analytics?.shutdown(); + } catch { + /* best-effort */ + } this.client.destroy(); await this.transport.close(); await this.server.close(); diff --git a/devtools/mcp/src/telemetry/__tests__/config.test.ts b/devtools/mcp/src/telemetry/__tests__/config.test.ts new file mode 100644 index 0000000..03e0fab --- /dev/null +++ b/devtools/mcp/src/telemetry/__tests__/config.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from "vitest"; + +import { resolveTelemetryConfig } from "../config"; + +const KEY = "phc_test_key"; + +describe("resolveTelemetryConfig", () => { + it("is disabled while the project key is an unreplaced placeholder", () => { + expect(resolveTelemetryConfig({})).toEqual({ enabled: false }); + }); + + it("is enabled with a configured key and defaults to the US host", () => { + expect( + resolveTelemetryConfig({ PLAYER_DEVTOOLS_TELEMETRY_KEY: KEY }), + ).toEqual({ + enabled: true, + apiKey: KEY, + host: "https://us.i.posthog.com", + }); + }); + + it.each([ + ["PLAYER_DEVTOOLS_TELEMETRY_DISABLED", "1"], + ["PLAYER_DEVTOOLS_TELEMETRY_DISABLED", "true"], + ["DO_NOT_TRACK", "1"], + ["DO_NOT_TRACK", "yes"], + ])("opts out when %s=%s", (name, value) => { + expect( + resolveTelemetryConfig({ + PLAYER_DEVTOOLS_TELEMETRY_KEY: KEY, + [name]: value, + }), + ).toEqual({ enabled: false }); + }); + + it.each([ + ["0", "explicit zero"], + ["false", "explicit false"], + ["", "empty string"], + ])("stays enabled for DO_NOT_TRACK=%s (%s)", (value) => { + expect( + resolveTelemetryConfig({ + PLAYER_DEVTOOLS_TELEMETRY_KEY: KEY, + DO_NOT_TRACK: value, + }), + ).toMatchObject({ enabled: true }); + }); + + it("normalizes trailing slashes on the host override", () => { + expect( + resolveTelemetryConfig({ + PLAYER_DEVTOOLS_TELEMETRY_KEY: KEY, + PLAYER_DEVTOOLS_TELEMETRY_HOST: "https://eu.i.posthog.com///", + }), + ).toMatchObject({ host: "https://eu.i.posthog.com" }); + }); +}); diff --git a/devtools/mcp/src/telemetry/__tests__/index.test.ts b/devtools/mcp/src/telemetry/__tests__/index.test.ts new file mode 100644 index 0000000..732f6aa --- /dev/null +++ b/devtools/mcp/src/telemetry/__tests__/index.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const instrument = vi.hoisted(() => vi.fn()); +const shutdown = vi.hoisted(() => vi.fn()); +const PostHogCtor = vi.hoisted(() => vi.fn()); +const getInstallId = vi.hoisted(() => vi.fn()); + +const INSTALL_ID = "3f2504e0-4f89-41d3-9a0c-0305e82c3301"; + +// Keep the identity deterministic — the real implementation touches the home +// directory, which is not writable under the Bazel test sandbox. +vi.mock("../installId", () => ({ getInstallId })); +vi.mock("@posthog/mcp", () => ({ instrument })); +vi.mock("posthog-node", () => ({ + PostHog: class { + shutdown = shutdown; + constructor(...args: Array) { + PostHogCtor(...args); + } + }, +})); + +const { createAnalytics } = await import("../index"); + +const KEY = "phc_test_key"; +const server = {} as object; +const deps = { isTransportConnected: () => true }; + +describe("createAnalytics", () => { + const original = { ...process.env }; + + beforeEach(() => { + vi.clearAllMocks(); + getInstallId.mockReturnValue(INSTALL_ID); + process.env.PLAYER_DEVTOOLS_TELEMETRY_KEY = KEY; + delete process.env.DO_NOT_TRACK; + delete process.env.PLAYER_DEVTOOLS_TELEMETRY_DISABLED; + }); + + afterEach(() => { + process.env = { ...original }; + }); + + it("does not instrument when telemetry is opted out", () => { + process.env.DO_NOT_TRACK = "1"; + + expect(createAnalytics(server, deps)).toBeNull(); + expect(instrument).not.toHaveBeenCalled(); + }); + + it("does not instrument without a configured key", () => { + delete process.env.PLAYER_DEVTOOLS_TELEMETRY_KEY; + + expect(createAnalytics(server, deps)).toBeNull(); + expect(instrument).not.toHaveBeenCalled(); + }); + + it("installs the property allowlist on the PostHog client itself", () => { + createAnalytics(server, deps); + + const [, options] = PostHogCtor.mock.calls[0] ?? []; + expect(options.before_send).toHaveLength(1); + + // The client-level hook must strip payloads regardless of the SDK. + const redacted = options.before_send[0]({ + event: "$mcp_tool_call", + properties: { $mcp_tool_name: "get_flow", $mcp_parameters: "SECRET" }, + }); + expect(JSON.stringify(redacted)).not.toContain("SECRET"); + }); + + it("disables the SDK's tool-schema-mutating context parameter", () => { + createAnalytics(server, deps); + + const [, , options] = instrument.mock.calls[0] ?? []; + expect(options.context).toBe(false); + }); + + it("identifies with the durable install id", () => { + createAnalytics(server, deps); + + const [, , options] = instrument.mock.calls[0] ?? []; + expect(options.identify.distinctId).toBe(INSTALL_ID); + // Person properties would de-anonymize the event stream. + expect(options.identify.properties).toBeUndefined(); + }); + + it("does not instrument when no durable identity is available", () => { + getInstallId.mockReturnValue(null); + + expect(createAnalytics(server, deps)).toBeNull(); + expect(instrument).not.toHaveBeenCalled(); + }); + + it("reports the settled transport state at capture time", () => { + let connected = false; + createAnalytics(server, { isTransportConnected: () => connected }); + + const [, , options] = instrument.mock.calls[0] ?? []; + expect(options.eventProperties().transport_connected).toBe(false); + + connected = true; + expect(options.eventProperties().transport_connected).toBe(true); + }); + + it("returns null instead of throwing when instrumentation fails", () => { + instrument.mockImplementationOnce(() => { + throw new Error("boom"); + }); + + expect(() => createAnalytics(server, deps)).not.toThrow(); + }); +}); diff --git a/devtools/mcp/src/telemetry/__tests__/installId.test.ts b/devtools/mcp/src/telemetry/__tests__/installId.test.ts new file mode 100644 index 0000000..0ac07f8 --- /dev/null +++ b/devtools/mcp/src/telemetry/__tests__/installId.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi } from "vitest"; + +import { getInstallId, type InstallIdDeps } from "../installId"; + +const UUID = "3f2504e0-4f89-41d3-9a0c-0305e82c3301"; + +const enoent = (): never => { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); +}; + +function setup(overrides: Partial = {}) { + const deps: InstallIdDeps = { + homedir: () => "/home/tester", + readFileSync: vi.fn(enoent) as unknown as InstallIdDeps["readFileSync"], + writeFileSync: vi.fn() as unknown as InstallIdDeps["writeFileSync"], + mkdirSync: vi.fn() as unknown as InstallIdDeps["mkdirSync"], + randomUUID: () => UUID, + ...overrides, + }; + return deps; +} + +const reads = (contents: string) => + vi.fn(() => contents) as unknown as InstallIdDeps["readFileSync"]; + +describe("getInstallId", () => { + it("mints and persists an id on first run", () => { + const deps = setup(); + + expect(getInstallId(deps)).toBe(UUID); + expect(deps.mkdirSync).toHaveBeenCalledWith( + "/home/tester/.player-ui-devtools", + { recursive: true }, + ); + expect(deps.writeFileSync).toHaveBeenCalledWith( + "/home/tester/.player-ui-devtools/install.json", + JSON.stringify({ installId: UUID }), + { mode: 0o600 }, + ); + }); + + it("reuses a persisted id without rewriting it", () => { + const existing = "11111111-2222-4333-8444-555555555555"; + const deps = setup({ + readFileSync: reads(JSON.stringify({ installId: existing })), + }); + + expect(getInstallId(deps)).toBe(existing); + expect(deps.writeFileSync).not.toHaveBeenCalled(); + }); + + it.each([ + ["corrupt JSON", "{ not json"], + ["non-string id", JSON.stringify({ installId: 42 })], + ["empty id", JSON.stringify({ installId: "" })], + ["non-uuid id", JSON.stringify({ installId: "nope" })], + ])("regenerates on %s", (_label, contents) => { + const deps = setup({ readFileSync: reads(contents) }); + + expect(getInstallId(deps)).toBe(UUID); + expect(deps.writeFileSync).toHaveBeenCalled(); + }); + + it("returns null when the directory cannot be created", () => { + const deps = setup({ + mkdirSync: vi.fn(() => { + throw Object.assign(new Error("EACCES"), { code: "EACCES" }); + }) as unknown as InstallIdDeps["mkdirSync"], + }); + + expect(getInstallId(deps)).toBeNull(); + }); + + it("returns null when the file cannot be written", () => { + const deps = setup({ + writeFileSync: vi.fn(() => { + throw Object.assign(new Error("EROFS"), { code: "EROFS" }); + }) as unknown as InstallIdDeps["writeFileSync"], + }); + + expect(getInstallId(deps)).toBeNull(); + }); +}); diff --git a/devtools/mcp/src/telemetry/__tests__/redaction.test.ts b/devtools/mcp/src/telemetry/__tests__/redaction.test.ts new file mode 100644 index 0000000..c91e84c --- /dev/null +++ b/devtools/mcp/src/telemetry/__tests__/redaction.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from "vitest"; + +import { + ALLOWED_EVENT_PROPERTIES, + allowlistEventProperties, +} from "../redaction"; + +const SECRET = "ZZZ-CANARY-ZZZ"; + +describe("allowlistEventProperties", () => { + it("strips captured tool parameters and responses", () => { + const result = allowlistEventProperties({ + event: "$mcp_tool_call", + properties: { + $mcp_tool_name: "invoke_action", + $mcp_duration_ms: 12, + $mcp_parameters: { + pluginId: "basic", + payload: { flowContent: SECRET }, + }, + $mcp_response: { data: SECRET }, + }, + }); + + // Assert on the serialized payload, not key presence — a nested leak would + // pass a shallow key check. + expect(JSON.stringify(result)).not.toContain(SECRET); + expect(result.properties).toEqual({ + $mcp_tool_name: "invoke_action", + $mcp_duration_ms: 12, + }); + }); + + it("drops unknown properties a future SDK version might add", () => { + const result = allowlistEventProperties({ + event: "$mcp_tool_call", + properties: { + $mcp_tool_name: "get_flow", + $mcp_raw_request: { body: SECRET }, + $mcp_arguments_v2: SECRET, + }, + }); + + expect(JSON.stringify(result)).not.toContain(SECRET); + expect(Object.keys(result.properties ?? {})).toEqual(["$mcp_tool_name"]); + }); + + it("preserves every allowlisted property", () => { + const properties = Object.fromEntries( + [...ALLOWED_EVENT_PROPERTIES].map((key) => [key, "kept"]), + ); + + const result = allowlistEventProperties({ + event: "$mcp_tool_call", + properties, + }); + + expect(Object.keys(result.properties ?? {}).sort()).toEqual( + [...ALLOWED_EVENT_PROPERTIES].sort(), + ); + }); + + it("yields empty properties rather than leaking when nothing is allowed", () => { + const result = allowlistEventProperties({ + event: "$mcp_tool_call", + properties: { $mcp_parameters: SECRET, $mcp_response: SECRET }, + }); + + expect(result.properties).toEqual({}); + expect(JSON.stringify(result)).not.toContain(SECRET); + }); + + it("tolerates events without properties", () => { + expect(() => + allowlistEventProperties({ event: "$mcp_initialize" }), + ).not.toThrow(); + }); + + it("passes through a null event dropped by an earlier hook", () => { + expect(allowlistEventProperties(null)).toBeNull(); + }); + + it("never allows the two payload keys, whatever else changes", () => { + expect(ALLOWED_EVENT_PROPERTIES.has("$mcp_parameters")).toBe(false); + expect(ALLOWED_EVENT_PROPERTIES.has("$mcp_response")).toBe(false); + }); +}); diff --git a/devtools/mcp/src/telemetry/config.ts b/devtools/mcp/src/telemetry/config.ts new file mode 100644 index 0000000..ef7fd16 --- /dev/null +++ b/devtools/mcp/src/telemetry/config.ts @@ -0,0 +1,51 @@ +/** + * PostHog project API key. This is a *public, write-only* ingestion token — it + * cannot read any data back out, which is why it is safe to ship in an + * open-source package. Replace the placeholder to enable telemetry. + */ +const POSTHOG_PROJECT_KEY = "phc_REPLACE_ME_WITH_REAL_PROJECT_KEY"; + +const DEFAULT_POSTHOG_HOST = "https://us.i.posthog.com"; + +export type TelemetryConfig = + | { readonly enabled: false } + | { readonly enabled: true; readonly apiKey: string; readonly host: string }; + +/** + * Presence of the variable opts out, *except* for explicit negatives — + * `DO_NOT_TRACK=0` means "tracking is fine", not "opted out". + */ +function isTruthy(value: string | undefined): boolean { + if (value === undefined) return false; + const normalized = value.trim().toLowerCase(); + if (normalized === "" || normalized === "0" || normalized === "false") { + return false; + } + return true; +} + +function isOptedOut(env: NodeJS.ProcessEnv): boolean { + return ( + isTruthy(env.PLAYER_DEVTOOLS_TELEMETRY_DISABLED) || + // Cross-vendor convention (consoledonottrack.com) — honored so a user who + // sets it once is covered everywhere. + isTruthy(env.DO_NOT_TRACK) + ); +} + +export function resolveTelemetryConfig( + env: NodeJS.ProcessEnv = process.env, +): TelemetryConfig { + if (isOptedOut(env)) return { enabled: false }; + + const apiKey = env.PLAYER_DEVTOOLS_TELEMETRY_KEY ?? POSTHOG_PROJECT_KEY; + // An unreplaced placeholder would fire doomed requests on every startup; + // treat it as "not configured" so the package is safe to ship unprovisioned. + if (!apiKey || apiKey.includes("REPLACE_ME")) return { enabled: false }; + + const host = ( + env.PLAYER_DEVTOOLS_TELEMETRY_HOST ?? DEFAULT_POSTHOG_HOST + ).replace(/\/+$/, ""); + + return { enabled: true, apiKey, host }; +} diff --git a/devtools/mcp/src/telemetry/index.ts b/devtools/mcp/src/telemetry/index.ts new file mode 100644 index 0000000..aefddc5 --- /dev/null +++ b/devtools/mcp/src/telemetry/index.ts @@ -0,0 +1,85 @@ +import * as os from "os"; +import { instrument } from "@posthog/mcp"; +import { PostHog } from "posthog-node"; + +import { resolveTelemetryConfig } from "./config"; +import { getInstallId } from "./installId"; +import { allowlistEventProperties } from "./redaction"; + +export { + ALLOWED_EVENT_PROPERTIES, + allowlistEventProperties, +} from "./redaction"; +export { resolveTelemetryConfig, type TelemetryConfig } from "./config"; +export { getInstallId, type InstallIdDeps } from "./installId"; + +declare global { + const __VERSION__: string; +} + +/** + * Stamped at release build time by tsup (see devtools/tsup.config.ts). The + * guard is required — the global is absent in local and test builds. + */ +export const MCP_VERSION: string = + typeof __VERSION__ !== "undefined" ? __VERSION__ : "unstamped"; + +export type AnalyticsDeps = { + /** Reports whether the devtools transport is currently connected. */ + isTransportConnected: () => boolean; +}; + +/** + * Instruments `server` with anonymous usage analytics. + * + * Returns the PostHog client so the caller can flush it on shutdown, or `null` + * when telemetry is disabled or no durable identity is available. Never throws: + * telemetry must not be able to prevent the server from starting. + */ +export function createAnalytics( + server: object, + deps: AnalyticsDeps, +): PostHog | null { + try { + const config = resolveTelemetryConfig(); + if (!config.enabled) return null; + + const installId = getInstallId(); + if (installId === null) return null; + + const posthog = new PostHog(config.apiKey, { + host: config.host, + // The load-bearing privacy control. Runs inside posthog-node's capture(), + // so it applies to everything the MCP SDK emits regardless of the SDK's + // own hooks. See redaction.ts for why this is an allowlist. + before_send: [allowlistEventProperties], + }); + + instrument(server, posthog, { + // Defaults to ON, and would inject a *required* `context` parameter into + // every tool schema asking the agent to narrate user intent — mutating + // our public tool API and adding per-call token cost. + context: false, + identify: { distinctId: installId }, + eventProperties: () => ({ + mcp_version: MCP_VERSION, + os_platform: os.platform(), + node_major: Number.parseInt( + process.versions.node.split(".")[0] ?? "", + 10, + ), + transport_connected: deps.isTransportConnected(), + surface: "mcp-server", + }), + // Redundant with `before_send` above, kept as a second independent layer. + // This hook fails closed: the SDK drops the event if it throws. + beforeSend: allowlistEventProperties, + }); + + return posthog; + } catch { + // Telemetry is strictly best-effort — a misconfiguration must never take + // down the server. + return null; + } +} diff --git a/devtools/mcp/src/telemetry/installId.ts b/devtools/mcp/src/telemetry/installId.ts new file mode 100644 index 0000000..5243372 --- /dev/null +++ b/devtools/mcp/src/telemetry/installId.ts @@ -0,0 +1,79 @@ +import { randomUUID } from "crypto"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +export type InstallIdDeps = { + /** Resolves the user's home directory. */ + homedir: () => string; + /** Reads the install file. */ + readFileSync: typeof fs.readFileSync; + /** Writes the install file. */ + writeFileSync: typeof fs.writeFileSync; + /** Creates the install directory. */ + mkdirSync: typeof fs.mkdirSync; + /** Mints a new identifier. */ + randomUUID: () => string; +}; + +const defaultDeps: InstallIdDeps = { + homedir: os.homedir, + readFileSync: fs.readFileSync, + writeFileSync: fs.writeFileSync, + mkdirSync: fs.mkdirSync, + randomUUID, +}; + +type InstallFile = { installId: string }; + +const UUID_PATTERN = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +export const INSTALL_DIR_NAME = ".player-ui-devtools"; +export const INSTALL_FILE_NAME = "install.json"; + +/** + * A stable, anonymous per-machine identifier. + * + * Stored under the user's home directory rather than the OS temp dir the + * flipper refcount uses: temp is periodically reaped, which would regenerate + * the id on every reboot and turn "unique installs" into "unique boots". + * + * Returns `null` when the id can neither be read nor persisted (read-only FS, + * sandbox, absent HOME). Callers treat that as "telemetry off" — returning an + * in-memory id instead would register a brand-new install on every run and + * systematically inflate counts for exactly the sandboxed population least + * likely to be real users. + */ +export function getInstallId(deps: InstallIdDeps = defaultDeps): string | null { + const dir = path.join(deps.homedir(), INSTALL_DIR_NAME); + const file = path.join(dir, INSTALL_FILE_NAME); + + try { + const parsed = JSON.parse( + deps.readFileSync(file, "utf8") as string, + ) as Partial; + // Validate the shape, not just the parse: `{"installId": 42}` parses fine + // but is not a usable identity. + if ( + typeof parsed.installId === "string" && + UUID_PATTERN.test(parsed.installId) + ) { + return parsed.installId; + } + } catch { + /* missing or corrupt — fall through and regenerate */ + } + + const installId = deps.randomUUID(); + try { + deps.mkdirSync(dir, { recursive: true }); + const contents: InstallFile = { installId }; + // 0600: a stable per-user identifier should not be world-readable. + deps.writeFileSync(file, JSON.stringify(contents), { mode: 0o600 }); + } catch { + return null; + } + + return installId; +} diff --git a/devtools/mcp/src/telemetry/redaction.ts b/devtools/mcp/src/telemetry/redaction.ts new file mode 100644 index 0000000..43fa0b2 --- /dev/null +++ b/devtools/mcp/src/telemetry/redaction.ts @@ -0,0 +1,72 @@ +/** + * Event property allowlist. + * + * `@posthog/mcp` captures tool arguments (`$mcp_parameters`) and tool results + * (`$mcp_response`) unconditionally — there is no SDK option to disable it, and + * its built-in sanitizer only matches sensitive *key names* (`token`, + * `password`, ...), so a field named `flowContent` would be transmitted + * verbatim. Devtools tool arguments carry player/plugin ids and `invoke_action` + * payloads, i.e. potentially customer flow data. + * + * This is an allowlist rather than a denylist on purpose: the SDK is 0.x and + * ships frequently, so a denylist naming today's two payload keys would + * silently start leaking the moment a release adds a third. Anything not + * explicitly listed here never reaches the wire. + */ +export const ALLOWED_EVENT_PROPERTIES: ReadonlySet = new Set([ + // MCP tool call shape — names and timings only, never payloads. + "$mcp_tool_name", + "$mcp_duration_ms", + "$mcp_is_error", + "$mcp_error_type", + "$mcp_listed_tool_names", + "$mcp_missing_capability", + // Which editor/agent is driving the server. + "$mcp_client_name", + "$mcp_client_version", + "$mcp_server_name", + "$mcp_server_version", + "$mcp_protocol_version", + // PostHog bookkeeping. + "$session_id", + "$process_person_profile", + "$lib", + "$lib_version", + // Our own event properties. + "mcp_version", + "os_platform", + "node_major", + "transport_connected", + "surface", +]); + +export type CapturedEvent = { + /** Event name. */ + event?: string; + /** Event properties. */ + properties?: Record; +}; + +/** + * Drops every property not in {@link ALLOWED_EVENT_PROPERTIES}. + * + * Installed as a `posthog-node` client-level `before_send`, which runs inside + * `capture()` — a chokepoint the MCP SDK cannot route around, since we own the + * client instance and it does not. + * + * Accepts `null` because `before_send` hooks compose: an earlier hook may have + * already dropped the event. + */ +export function allowlistEventProperties( + event: Event | null, +): Event | null { + if (!event?.properties) return event; + + event.properties = Object.fromEntries( + Object.entries(event.properties).filter(([key]) => + ALLOWED_EVENT_PROPERTIES.has(key), + ), + ); + + return event; +} diff --git a/package.json b/package.json index a4327bd..363fcd0 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "@player-ui/reference-assets-plugin-react": ">=0.15.4", "@player-ui/storybook": ">=0.15.4", "@player-ui/types": ">=0.15.4", + "@posthog/mcp": "0.10.8", "@storybook/addon-docs": "^7.6.10", "@storybook/blocks": "^7.6.10", "@storybook/builder-vite": "^7.6.10", @@ -117,6 +118,7 @@ "log-update": "^4.0.0", "mkdirp": "^1.0.4", "oclif": "^4.4.2", + "posthog-node": "^5.0.0", "prettier": "^3.5.3", "react": "^18.2.0", "react-dom": "^18.2.0", @@ -144,11 +146,11 @@ }, "packageManager": "pnpm@10.12.1", "engines": { - "node": "^22.12.0", + "node": ">=22.22.0", "pnpm": "^10.12.1" }, "volta": { - "node": "22.12.0", + "node": "22.22.0", "pnpm": "10.12.1" }, "pnpm": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f7b0cf7..7a656ed 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -115,6 +115,9 @@ importers: '@player-ui/types': specifier: '>=0.15.4' version: 1.0.1 + '@posthog/mcp': + specifier: 0.10.8 + version: 0.10.8(@modelcontextprotocol/sdk@1.29.0(zod@3.25.76))(posthog-node@5.48.1(rxjs@7.8.2)) '@storybook/addon-docs': specifier: ^7.6.10 version: 7.6.20(@types/react-dom@18.3.7(@types/react@18.3.24))(@types/react@18.3.24)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -319,6 +322,9 @@ importers: oclif: specifier: ^4.4.2 version: 4.22.22(@types/node@24.5.2) + posthog-node: + specifier: ^5.0.0 + version: 5.48.1(rxjs@7.8.2) prettier: specifier: ^3.5.3 version: 3.6.2 @@ -3109,19 +3115,23 @@ packages: '@modelcontextprotocol/inspector-cli@0.21.2': resolution: {integrity: sha512-Om2ApfIbkbfR7+PqJX0bO2Qwg0z55MgxquC+G0YL6x80ElpZMfxITsWQ1238kh3bv1zlwPSePc2kvDnCcU5tXw==, tarball: https://registry.npmjs.org/@modelcontextprotocol/inspector-cli/-/inspector-cli-0.21.2.tgz} + deprecated: 'v1 is deprecated. Upgrade to v2: npm i @modelcontextprotocol/inspector@latest. v1 gets security fixes only, published under the v1-latest tag.' hasBin: true '@modelcontextprotocol/inspector-client@0.21.2': resolution: {integrity: sha512-8us3hVXDgMHGb5jF1bBE/a6rRRlEaS+SKjbDFB56oE6+mNcAP9JgL8h6T0fYd3XQ3vL/CYxjVeQE+5yRdvhsVg==, tarball: https://registry.npmjs.org/@modelcontextprotocol/inspector-client/-/inspector-client-0.21.2.tgz} + deprecated: 'v1 is deprecated. Upgrade to v2: npm i @modelcontextprotocol/inspector@latest. v1 gets security fixes only, published under the v1-latest tag.' hasBin: true '@modelcontextprotocol/inspector-server@0.21.2': resolution: {integrity: sha512-ASFNPFMnT0Vn5u6aYRwwMrwA/0qpxb1lg3sE5GjWii5bUfPNXuIaLOXuXKSOFIMG4o82RxpuipdqX1ZdsmTPUw==, tarball: https://registry.npmjs.org/@modelcontextprotocol/inspector-server/-/inspector-server-0.21.2.tgz} + deprecated: 'v1 is deprecated. Upgrade to v2: npm i @modelcontextprotocol/inspector@latest. v1 gets security fixes only, published under the v1-latest tag.' hasBin: true '@modelcontextprotocol/inspector@0.21.2': resolution: {integrity: sha512-f/zIzl6ccYjIxSTgmol9EKvd8AXsXkIaZX16kLGhrYwxrApvU3IL6IzgOqATKP/2kgyKdFUOVLlHMxX9e6BZZA==, tarball: https://registry.npmjs.org/@modelcontextprotocol/inspector/-/inspector-0.21.2.tgz} engines: {node: '>=22.7.5'} + deprecated: 'v1 is deprecated. Upgrade to v2: npm i @modelcontextprotocol/inspector@latest. v1 gets security fixes only, published under the v1-latest tag.' hasBin: true '@modelcontextprotocol/sdk@1.29.0': @@ -3748,6 +3758,19 @@ packages: '@popperjs/core@2.11.8': resolution: {integrity: sha512-P1st0aksCrn9sGZhp8GMYwBnQsbvAWsZAX44oXNNvLHGqAOcoVxmjZiohstwQ7SqKnbR47akdNi+uleWD8+g6A==, tarball: https://registry.npmjs.org/@popperjs/core/-/core-2.11.8.tgz} + '@posthog/core@1.46.9': + resolution: {integrity: sha512-EXO6y5ih+jBkTCpUCuYgQmajuDZuvy6vMvflkub6pLQyi0GPlCPWVSvZZkOeQw9e2MxoD5GteeGCt9R8+UJ/yQ==, tarball: https://registry.npmjs.org/@posthog/core/-/core-1.46.9.tgz} + + '@posthog/mcp@0.10.8': + resolution: {integrity: sha512-p3beWalPLashncxN1Q5XCrQgEn87JDToGTLBL2ThLMTREgxllz2yNEbnjpAfBlKYCP/RII9EYJ6UEL8IHT6hNQ==, tarball: https://registry.npmjs.org/@posthog/mcp/-/mcp-0.10.8.tgz} + engines: {node: ^20.20.0 || >=22.22.0} + peerDependencies: + '@modelcontextprotocol/sdk': '>=1.26.0' + posthog-node: ^5.0.0 + + '@posthog/types@1.402.2': + resolution: {integrity: sha512-ZZTiS4dLwF4/D0YTzS3gGSLFnhuNOY5yu4d9VGS9trGe5GW6FjIXo20p18K5BPW2RZSYSld8sdnSAY3AUXleUQ==, tarball: https://registry.npmjs.org/@posthog/types/-/types-1.402.2.tgz} + '@radix-ui/number@1.0.1': resolution: {integrity: sha512-T5gIdVO2mmPW3NNhjNgEP3cqMXjXL9UbO0BzWcXfvdBs+BohbQxvd/K5hSVKmn9/lbTdsQVKbUcP5WLCwvUbBg==, tarball: https://registry.npmjs.org/@radix-ui/number/-/number-1.0.1.tgz} @@ -9723,6 +9746,15 @@ packages: resolution: {integrity: sha512-3Ybi1tAuwAP9s0r1UQ2J4n5Y0G05bJkpUIO0/bI9MhwmD70S5aTWbXGBwxHrelT+XM1k6dM0pk+SwNkpTRN7Pg==, tarball: https://registry.npmjs.org/postcss/-/postcss-8.5.6.tgz} engines: {node: ^10 || ^12 || >=14} + posthog-node@5.48.1: + resolution: {integrity: sha512-BxLX2SqGEQhPqCPTalpyo0RRv1NMbf7UaN7q9d/ED77ksD6XOmE7ko2vIKO8F0zPL1NtKxIi+DYXap9lvR0RaA==, tarball: https://registry.npmjs.org/posthog-node/-/posthog-node-5.48.1.tgz} + engines: {node: ^20.20.0 || >=22.22.0} + peerDependencies: + rxjs: ^7.0.0 + peerDependenciesMeta: + rxjs: + optional: true + prelude-ls@1.2.1: resolution: {integrity: sha512-vkcDPrRZo1QZLbn5RLGPpg/WmIQ65qoWWhcGKf/b5eplkkarX0m9z8ppCat4mlOqUsWpyNuYgO3VRyrYHSzX5g==, tarball: https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.2.1.tgz} engines: {node: '>= 0.8.0'} @@ -16819,6 +16851,18 @@ snapshots: '@popperjs/core@2.11.8': {} + '@posthog/core@1.46.9': + dependencies: + '@posthog/types': 1.402.2 + + '@posthog/mcp@0.10.8(@modelcontextprotocol/sdk@1.29.0(zod@3.25.76))(posthog-node@5.48.1(rxjs@7.8.2))': + dependencies: + '@modelcontextprotocol/sdk': 1.29.0(zod@3.25.76) + '@posthog/core': 1.46.9 + posthog-node: 5.48.1(rxjs@7.8.2) + + '@posthog/types@1.402.2': {} + '@radix-ui/number@1.0.1': dependencies: '@babel/runtime': 7.28.4 @@ -24296,6 +24340,12 @@ snapshots: picocolors: 1.1.1 source-map-js: 1.2.1 + posthog-node@5.48.1(rxjs@7.8.2): + dependencies: + '@posthog/core': 1.46.9 + optionalDependencies: + rxjs: 7.8.2 + prelude-ls@1.2.1: {} prettier-linter-helpers@1.0.0: From 1ab94ed9e6fcf34830abd4cd93a37334daa7d530 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Thu, 6 Aug 2026 20:06:21 -0700 Subject: [PATCH 2/3] Stamp the PostHog key at build time instead of hardcoding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors how __VERSION__ is stamped: workspace-status.sh emits STABLE_POSTHOG_KEY from the POSTHOG_PROJECT_KEY environment variable, and tsup substitutes it into the __POSTHOG_KEY__ global. CI supplies the value, so no key lives in the repo. Unstamped builds (local, tests, non-release CI) resolve the global to an empty string and telemetry stays disabled — verified in the emitted bundle: a release build inlines `POSTHOG_PROJECT_KEY = "phc_..."` while a normal build keeps the runtime typeof guard. Also refuses any key that is not a `phc_` public project token. A `phx_` personal or `phs_` project-secret key is a real credential, and stamped values land in build artifacts and the shared BuildBuddy cache; if one is ever wired into CI by mistake the build ships inert rather than leaking it. --- .../src/telemetry/__tests__/config.test.ts | 14 ++++++++- devtools/mcp/src/telemetry/config.ts | 31 ++++++++++++++----- helpers/release/workspace-status.sh | 15 ++++++++- tsup.config.ts | 1 + 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/devtools/mcp/src/telemetry/__tests__/config.test.ts b/devtools/mcp/src/telemetry/__tests__/config.test.ts index 03e0fab..dd16999 100644 --- a/devtools/mcp/src/telemetry/__tests__/config.test.ts +++ b/devtools/mcp/src/telemetry/__tests__/config.test.ts @@ -5,10 +5,22 @@ import { resolveTelemetryConfig } from "../config"; const KEY = "phc_test_key"; describe("resolveTelemetryConfig", () => { - it("is disabled while the project key is an unreplaced placeholder", () => { + it("is disabled in unstamped builds, where no key is baked in", () => { expect(resolveTelemetryConfig({})).toEqual({ enabled: false }); }); + it.each([ + ["phx_personal_key", "personal API key"], + ["phs_project_secret", "project secret key"], + ["not-a-posthog-key", "malformed value"], + ])("refuses to use %s (%s)", (key) => { + // A non-public key is a real credential; shipping one would be worse than + // shipping no telemetry at all. + expect( + resolveTelemetryConfig({ PLAYER_DEVTOOLS_TELEMETRY_KEY: key }), + ).toEqual({ enabled: false }); + }); + it("is enabled with a configured key and defaults to the US host", () => { expect( resolveTelemetryConfig({ PLAYER_DEVTOOLS_TELEMETRY_KEY: KEY }), diff --git a/devtools/mcp/src/telemetry/config.ts b/devtools/mcp/src/telemetry/config.ts index ef7fd16..78c1c44 100644 --- a/devtools/mcp/src/telemetry/config.ts +++ b/devtools/mcp/src/telemetry/config.ts @@ -1,9 +1,22 @@ +declare global { + const __POSTHOG_KEY__: string; +} + /** - * PostHog project API key. This is a *public, write-only* ingestion token — it - * cannot read any data back out, which is why it is safe to ship in an - * open-source package. Replace the placeholder to enable telemetry. + * PostHog project API key, stamped at release build time from the + * `POSTHOG_PROJECT_KEY` CI environment variable (see + * helpers/release/workspace-status.sh and tsup.config.ts), mirroring how + * `__VERSION__` is stamped. + * + * This is a *public, write-only* ingestion token — it can only send events and + * cannot read data back, which is why it is safe to ship in an open-source + * package. + * + * The `typeof` guard is required: the global is absent in local and test + * builds, where it resolves to an empty string and disables telemetry. */ -const POSTHOG_PROJECT_KEY = "phc_REPLACE_ME_WITH_REAL_PROJECT_KEY"; +const POSTHOG_PROJECT_KEY = + typeof __POSTHOG_KEY__ !== "undefined" ? __POSTHOG_KEY__ : ""; const DEFAULT_POSTHOG_HOST = "https://us.i.posthog.com"; @@ -39,9 +52,13 @@ export function resolveTelemetryConfig( if (isOptedOut(env)) return { enabled: false }; const apiKey = env.PLAYER_DEVTOOLS_TELEMETRY_KEY ?? POSTHOG_PROJECT_KEY; - // An unreplaced placeholder would fire doomed requests on every startup; - // treat it as "not configured" so the package is safe to ship unprovisioned. - if (!apiKey || apiKey.includes("REPLACE_ME")) return { enabled: false }; + // Unstamped (local/dev builds) or unset in CI — stay silent rather than + // firing doomed requests at PostHog on every startup. + if (!apiKey) return { enabled: false }; + // Only public write-only project keys may ship. A `phx_` (personal) or + // `phs_` (project secret) key is a real credential and must never be baked + // into a published artifact, so refuse to use one even if stamped. + if (!apiKey.startsWith("phc_")) return { enabled: false }; const host = ( env.PLAYER_DEVTOOLS_TELEMETRY_HOST ?? DEFAULT_POSTHOG_HOST diff --git a/helpers/release/workspace-status.sh b/helpers/release/workspace-status.sh index 181002c..67960ac 100755 --- a/helpers/release/workspace-status.sh +++ b/helpers/release/workspace-status.sh @@ -12,4 +12,17 @@ git_branch=$(git rev-parse --abbrev-ref HEAD) echo "GIT_BRANCH $git_branch" git_tree_status=$(git diff-index --quiet HEAD -- && echo 'Clean' || echo 'Modified') -echo "GIT_TREE_STATUS $git_tree_status" \ No newline at end of file +echo "GIT_TREE_STATUS $git_tree_status" + +# PostHog telemetry ingestion key, supplied by the CI environment. +# +# This is a *public, write-only* project key (`phc_...`): it can only send +# events and cannot read anything back, which is why it is safe to bake into a +# published package. Do not put a personal (`phx_`) or project-secret (`phs_`) +# key here — those are real credentials and this value is stamped into build +# artifacts and the shared remote cache. +# +# Emitted unconditionally so the key is part of the cache key: a changed key +# must invalidate stamped artifacts rather than leave a stale one cached. When +# unset the build stamps an empty string and telemetry stays disabled. +echo "STABLE_POSTHOG_KEY ${POSTHOG_PROJECT_KEY:-}" \ No newline at end of file diff --git a/tsup.config.ts b/tsup.config.ts index 3a1c0cf..9dbdfe7 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -31,6 +31,7 @@ function getStampedSubstitutions(): Record { const customSubstitutions = { __VERSION__: "{STABLE_VERSION}", __GIT_COMMIT__: "{STABLE_GIT_COMMIT}", + __POSTHOG_KEY__: "{STABLE_POSTHOG_KEY}", }; const substitutions: Record = {}; From dc9b8a14c219ed667c9482186ee2953839e0c4a5 Mon Sep 17 00:00:00 2001 From: Jeremiah Zucker Date: Thu, 6 Aug 2026 20:11:18 -0700 Subject: [PATCH 3/3] Document telemetry as zero-config for users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ingestion key is a build/CI concern, so the user-facing section now says so explicitly — no account, no key, no configuration — and notes that non-release builds send nothing, which is the likely first question when someone builds locally and sees no events. Moves the key plumbing into a collapsed maintainer note: which environment variable CI sets, how it reaches the bundle, and why only public `phc_` keys are accepted. Also corrects the event list to the names actually emitted ($mcp_initialize, $mcp_tool_call, $mcp_tools_list, $exception), verified against a captured session rather than the SDK's constants. --- devtools/mcp/README.md | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/devtools/mcp/README.md b/devtools/mcp/README.md index 5216133..6710eb3 100644 --- a/devtools/mcp/README.md +++ b/devtools/mcp/README.md @@ -143,7 +143,7 @@ and whether it's working in the field. It is **on by default** and sends: | | | | --- | --- | | Identity | A random UUID generated on first run and stored at `~/.player-ui-devtools/install.json`. It is not derived from anything about you or your machine — delete the file and a new one is generated. | -| Events | Server start, tool call, tools list, and errors. | +| Events | Session start (`$mcp_initialize`), tool calls (`$mcp_tool_call`), tool listing (`$mcp_tools_list`), and errors (`$exception`). | | Properties | Tool **name**, call duration, whether the call errored, the MCP client name/version (e.g. which editor), the devtools version, OS platform, Node major version, and whether the Flipper transport connected. | **Tool arguments and tool responses are never transmitted.** Those can contain @@ -162,4 +162,31 @@ export DO_NOT_TRACK=1 `DO_NOT_TRACK=0` and `DO_NOT_TRACK=false` are treated as "tracking is fine", not as an opt-out. +Nothing else is needed to make this work — no account, no key, no configuration. +Builds you make yourself (anything not a tagged release) send nothing at all. + +
+Maintainers: how the ingestion key is supplied + +The PostHog key is stamped into released builds; it is not in the repo and is +not something users provide. + +Set `POSTHOG_PROJECT_KEY` in the release CI environment. +[`helpers/release/workspace-status.sh`](../../helpers/release/workspace-status.sh) +emits it as `STABLE_POSTHOG_KEY`, and +[`tsup.config.ts`](../../tsup.config.ts) substitutes it into the +`__POSTHOG_KEY__` global — the same mechanism that stamps `__VERSION__`. + +Stamping only happens under `--config=release`, so PR and local builds resolve +the global to an empty string and stay silent. Only public `phc_` project keys +are accepted: the value is baked into published artifacts and the shared remote +cache, so a `phx_` personal or `phs_` secret key is rejected at runtime rather +than shipped. + +To point a build at a different project or region without rebuilding, override +`PLAYER_DEVTOOLS_TELEMETRY_KEY` / `PLAYER_DEVTOOLS_TELEMETRY_HOST` at runtime — +useful for verifying against a local listener. + +
+ [browser extension]: https://github.com/player-ui/browser-devtools