From cfa5bc2cf5a401d62d3dca72a9e1aa82f96d9506 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:56:38 -0400 Subject: [PATCH] Gate GA4 MCP tools per-user; fix false ga4_malformed_response on empty comparison periods (#497) --- README.md | 2 +- .../Ga4OrganicOverviewService.test.ts | 44 ++++++++++ .../ga4/services/Ga4ReportNormalization.ts | 16 +++- .../Ga4ReportingService.comparison.test.ts | 87 +++++++++++++++++++ src/server/mcp/server.ts | 8 +- 5 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 src/server/features/ga4/services/Ga4ReportingService.comparison.test.ts diff --git a/README.md b/README.md index 13c5489..9a874a2 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ OpenSEO supports two self-hosting paths: - **Simple: Docker (Best for testing it out)** - For personal use on your own machine. See [`docs/SELF_HOSTING_DOCKER.md`](./docs/SELF_HOSTING_DOCKER.md). - Unless you already are self-hosting other apps and are confident doing so, we recommend self-hosting with Cloudflare as opposed to Railway, Coolify or Dokploy. - - We plan to make it simpler to host on those platforms in the next few months. + - We plan to make it simpler to host on those platforms in the next few months. - **Recommended: Cloudflare** - For internet-facing self-hosting across multiple devices or with your team (works on the free plan). See [`docs/SELF_HOSTING_CLOUDFLARE.md`](./docs/SELF_HOSTING_CLOUDFLARE.md). Either way, you need a DataForSEO API key to get SEO data. See [`docs/DATAFORSEO_API_KEY.md`](./docs/DATAFORSEO_API_KEY.md). diff --git a/src/server/features/ga4/services/Ga4OrganicOverviewService.test.ts b/src/server/features/ga4/services/Ga4OrganicOverviewService.test.ts index f2cfa76..10bd0f7 100644 --- a/src/server/features/ga4/services/Ga4OrganicOverviewService.test.ts +++ b/src/server/features/ga4/services/Ga4OrganicOverviewService.test.ts @@ -123,6 +123,50 @@ describe("Ga4OrganicOverviewService", () => { expect(mocks.runReport).toHaveBeenCalledTimes(3); }); + it("treats a headerless previous-period response as empty instead of malformed", async () => { + mocks.runReport + .mockResolvedValueOnce({ + dimensionHeaders: [], + metricHeaders, + rows: [ + { + dimensionValues: [], + metricValues: metricValues([ + "100", + "80", + "70", + "0.7", + "10", + "4", + "500", + ]), + }, + ], + rowCount: 1, + }) + // GA4 omits headers and rows entirely when the previous-period window + // falls before the property's creation date. + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({ + dimensionHeaders: [{ name: "date" }], + metricHeaders, + rows: [], + rowCount: 0, + }); + const result = await Ga4OrganicOverviewService.getOrganicOverview( + { projectId: "project_1", trend: "daily" }, + { now: new Date("2026-08-06T15:00:00Z") }, + ); + expect(result.previous).toBeNull(); + expect(result.comparison.sessions).toEqual({ + current: 100, + previous: null, + absoluteChange: null, + percentChange: null, + }); + expect(result.diagnostics).toEqual([]); + }); + it("flags a material key-event decline with explicit evidence", async () => { const report = (keyEvents: string) => ({ dimensionHeaders: [], diff --git a/src/server/features/ga4/services/Ga4ReportNormalization.ts b/src/server/features/ga4/services/Ga4ReportNormalization.ts index 556c9ff..2f60c1e 100644 --- a/src/server/features/ga4/services/Ga4ReportNormalization.ts +++ b/src/server/features/ga4/services/Ga4ReportNormalization.ts @@ -50,9 +50,21 @@ export function normalizeGa4Response( const expectedMetrics = request.metrics.map(({ name }) => name); const dimensions = (response.dimensionHeaders ?? []).map(({ name }) => name); const metrics = (response.metricHeaders ?? []).map(({ name }) => name); + + // GA4 omits headers and rows entirely (rather than echoing the requested + // headers with zero rows) when the date range has no data on record for + // the property at all — e.g. a previous-period comparison window that + // falls before the property's creation date. Treat that as a + // legitimately empty report instead of a malformed response. + const isHeaderlessEmptyResponse = + response.dimensionHeaders === undefined && + response.metricHeaders === undefined && + (response.rows?.length ?? 0) === 0; + if ( - dimensions.join("\0") !== expectedDimensions.join("\0") || - metrics.join("\0") !== expectedMetrics.join("\0") + !isHeaderlessEmptyResponse && + (dimensions.join("\0") !== expectedDimensions.join("\0") || + metrics.join("\0") !== expectedMetrics.join("\0")) ) { throw new Ga4MalformedResponseError(); } diff --git a/src/server/features/ga4/services/Ga4ReportingService.comparison.test.ts b/src/server/features/ga4/services/Ga4ReportingService.comparison.test.ts new file mode 100644 index 0000000..3ca9fc8 --- /dev/null +++ b/src/server/features/ga4/services/Ga4ReportingService.comparison.test.ts @@ -0,0 +1,87 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { + Ga4RunReportRequest, + Ga4RunReportResponse, +} from "@/server/lib/ga4Client"; +import { makeGa4Connection } from "./ga4-test-fixtures"; +import { Ga4ReportingService } from "./Ga4ReportingService"; + +const mocks = vi.hoisted(() => ({ + getByProjectId: vi.fn(), + runReport: + vi.fn<(request: Ga4RunReportRequest) => Promise>(), +})); + +vi.mock("@/server/features/ga4/repositories/Ga4ConnectionRepository", () => ({ + Ga4ConnectionRepository: { getByProjectId: mocks.getByProjectId }, +})); + +vi.mock("@/server/lib/ga4Client", () => ({ + createGa4DataClient: () => ({ runReport: mocks.runReport }), +})); + +const connection = makeGa4Connection(); + +function noComparison() { + return { + current: null, + previous: null, + absoluteChange: null, + percentChange: null, + }; +} + +describe("Ga4ReportingService previous-period comparison", () => { + beforeEach(() => { + mocks.getByProjectId.mockResolvedValue(connection); + }); + + it("treats a headerless previous-period response as empty instead of malformed", async () => { + mocks.runReport + .mockResolvedValueOnce({ + dimensionHeaders: [{ name: "deviceCategory" }], + metricHeaders: [ + "activeUsers", + "sessions", + "engagementRate", + "keyEvents", + ].map((name) => ({ name })), + rows: [ + { + dimensionValues: [{ value: "mobile" }], + metricValues: ["10", "12", "0.5", "2"].map((value) => ({ value })), + }, + ], + rowCount: 1, + }) + // GA4 omits headers and rows entirely when the previous-period window + // falls before the property's creation date. + .mockResolvedValueOnce({}); + + const result = await Ga4ReportingService.runReport( + { + projectId: "project_1", + kind: "audience_breakdown", + audienceBreakdown: "device", + comparePreviousPeriod: true, + }, + { now: new Date("2026-08-06T15:00:00Z") }, + ); + + expect(result.comparison?.rows).toEqual([ + { + dimensions: { deviceCategory: "mobile" }, + metrics: { + activeUsers: { ...noComparison(), current: 10 }, + sessions: { ...noComparison(), current: 12 }, + engagementRate: { ...noComparison(), current: 0.5 }, + keyEvents: { ...noComparison(), current: 2 }, + }, + }, + ]); + expect(result.comparison?.coverage.previous).toEqual({ + fetchedRowCount: 0, + totalRowCount: 0, + }); + }); +}); diff --git a/src/server/mcp/server.ts b/src/server/mcp/server.ts index 2b61d61..c3ada49 100644 --- a/src/server/mcp/server.ts +++ b/src/server/mcp/server.ts @@ -6,6 +6,7 @@ import { import type { z } from "zod"; import { createMcpToolContext, + MCP_AUTH_CONTEXT_PROP, type McpProps, type ToolContext, } from "@/server/mcp/context"; @@ -58,7 +59,7 @@ import { getSearchConsolePerformanceTool, inspectUrlsTool, } from "@/server/mcp/tools/search-console-tools"; -import { GA4_OAUTH_APP_PENDING } from "@/shared/ga4"; +import { GA4_OAUTH_APP_PENDING, isGa4ConnectAvailable } from "@/shared/ga4"; import { getAuditIssuesTool, getAuditPagesTool, @@ -179,7 +180,10 @@ export function createOpenSeoMcpServer(authProps: McpProps) { register(getKeywordMetricsTool); register(getSearchConsolePerformanceTool); register(inspectUrlsTool); - if (!GA4_OAUTH_APP_PENDING) { + if ( + !GA4_OAUTH_APP_PENDING || + isGa4ConnectAvailable(authProps[MCP_AUTH_CONTEXT_PROP].userEmail) + ) { register(getGoogleAnalyticsOrganicLandingPagesTool); register(getGoogleAnalyticsPagePerformanceTool); register(getGoogleAnalyticsKeyEventsTool);