From 123538caca7b2d4558a84ed58707431ce1a4b371 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 25 Jul 2026 08:24:04 -0700 Subject: [PATCH] fix(posthog): pass release-name and release-version as separate CLI flags posthog-cli sourcemap inject/upload combine --release-name and --release-version server-side into "{name}@{version}". Every call site (self-host's release workflow, review-enrichment, discovery-index, and the self-host operator deploy script) only passed our own already-combined string as --release-version, leaving --release-name unset. The CLI then auto-derived its own name from git/package.json, silently doubling the stored release id and breaking release-based symbol-set lookups. Also wires self-host's previously-computed-but-unused resolvePostHogRelease into its captured PostHog events, matching how review-enrichment and discovery-index already tag captures with their release. --- .github/workflows/release-selfhost.yml | 12 ++++++-- .../discovery-index/src/upload-sourcemaps.ts | 17 +++++++++-- review-enrichment/src/upload-sourcemaps.ts | 17 +++++++++-- review-enrichment/test/posthog-upload.test.ts | 29 +++++++++++++++++-- scripts/deploy-selfhost-prebuilt.sh | 13 +++++++-- src/selfhost/posthog.ts | 6 ++++ .../discovery-index/upload-sourcemaps.test.ts | 15 ++++++++-- test/unit/selfhost-posthog-release.test.ts | 14 +++++++-- test/unit/selfhost-posthog.test.ts | 24 +++++++++++++++ 9 files changed, 130 insertions(+), 17 deletions(-) diff --git a/.github/workflows/release-selfhost.yml b/.github/workflows/release-selfhost.yml index 8aa9f744c..714a525dd 100644 --- a/.github/workflows/release-selfhost.yml +++ b/.github/workflows/release-selfhost.yml @@ -161,7 +161,13 @@ jobs: POSTHOG_CLI_API_KEY: ${{ secrets.POSTHOG_CLI_API_KEY }} POSTHOG_CLI_PROJECT_ID: ${{ secrets.POSTHOG_CLI_PROJECT_ID }} POSTHOG_CLI_HOST: ${{ secrets.POSTHOG_CLI_HOST }} - POSTHOG_RELEASE: ${{ steps.version.outputs.release }} + # Passed as separate --release-name/--release-version flags below rather than one combined + # --release-version string: posthog-cli otherwise auto-derives its own release-name from git/ + # package.json (this repo resolves to "loopover") and prepends it, so an unqualified + # --release-version "loopover-orb@$VERSION" silently becomes the stored release + # "loopover@loopover-orb@$VERSION" -- never matching what "Validate PostHog release" below looks up. + POSTHOG_RELEASE_NAME: loopover-orb + POSTHOG_RELEASE_VERSION: ${{ steps.version.outputs.v }} run: | set -euo pipefail test -n "$POSTHOG_CLI_API_KEY" @@ -172,9 +178,9 @@ jobs: if [ -z "${POSTHOG_CLI_HOST:-}" ]; then unset POSTHOG_CLI_HOST; fi # No separate "create release" step -- PostHog release metadata is a byproduct of the inject/ # upload calls below, unlike Sentry's releases/commits/deploys/finalize lifecycle this replaces. - npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-version "$POSTHOG_RELEASE" + npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION" node --experimental-strip-types scripts/validate-selfhost-sourcemap.ts - npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-version "$POSTHOG_RELEASE" + npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION" - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4 - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4 diff --git a/packages/discovery-index/src/upload-sourcemaps.ts b/packages/discovery-index/src/upload-sourcemaps.ts index b3020a7cf..43c90a017 100644 --- a/packages/discovery-index/src/upload-sourcemaps.ts +++ b/packages/discovery-index/src/upload-sourcemaps.ts @@ -25,6 +25,18 @@ function nonBlank(value: string | undefined): string | undefined { return text ? text : undefined; } +// posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags, which +// it combines server-side into the release id "{name}@{version}". Passing our own already-combined +// POSTHOG_RELEASE (e.g. "loopover-discovery-index@") as --release-version alone leaves --release-name +// unset, and the CLI then auto-derives one from git/package.json instead -- silently doubling up into +// "@loopover-discovery-index@", which never matches what runReleaseValidation looks up. +// Splitting our own convention at its first "@" reproduces exactly the release id we already expect. +function splitRelease(release: string, defaultName: string): { name: string; version: string } { + const at = release.indexOf("@"); + if (at === -1) return { name: defaultName, version: release }; + return { name: release.slice(0, at), version: release.slice(at + 1) }; +} + function log(event: string, fields: Record = {}): void { console.log(JSON.stringify({ event, ...fields })); } @@ -161,9 +173,10 @@ async function main(): Promise { // calls below. Explicit --release-version rather than posthog-cli's own git-metadata auto-detection: the // Dockerfile's build stage only copies packages/loopover-engine and packages/discovery-index source, // never `.git`, so auto-detection has nothing to inspect at container-startup time. - runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-version", release!]); + const { name: releaseName, version: releaseVersion } = splitRelease(release!, "loopover-discovery-index"); + runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]); validateSourceMaps(); - runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-version", release!]); + runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]); await runReleaseValidation(release!); log("discovery_index_posthog_sourcemap_upload_complete", { release }); return 0; diff --git a/review-enrichment/src/upload-sourcemaps.ts b/review-enrichment/src/upload-sourcemaps.ts index 453212478..b33fa4acf 100644 --- a/review-enrichment/src/upload-sourcemaps.ts +++ b/review-enrichment/src/upload-sourcemaps.ts @@ -28,6 +28,18 @@ function nonBlank(value: string | undefined): string | undefined { return text ? text : undefined; } +// posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags, which +// it combines server-side into the release id "{name}@{version}". Passing our own already-combined +// POSTHOG_RELEASE (e.g. "loopover-rees@") as --release-version alone leaves --release-name unset, and +// the CLI then auto-derives one from git/package.json instead -- silently doubling up into +// "@loopover-rees@", which never matches what runReleaseValidation looks up. Splitting +// our own convention at its first "@" reproduces exactly the release id we already expect. +function splitRelease(release: string, defaultName: string): { name: string; version: string } { + const at = release.indexOf("@"); + if (at === -1) return { name: defaultName, version: release }; + return { name: release.slice(0, at), version: release.slice(at + 1) }; +} + function log(event: string, fields: Record = {}): void { console.log(JSON.stringify({ event, ...fields })); } @@ -166,9 +178,10 @@ async function main(): Promise { // No separate "create release" step (PostHog release metadata is a byproduct of inject/upload) and an // explicit --release-version rather than posthog-cli's own git-metadata auto-detection -- the deploy // environment isn't guaranteed to have a usable .git checkout at this step. - runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-version", release!]); + const { name: releaseName, version: releaseVersion } = splitRelease(release!, "loopover-rees"); + runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]); validateSourceMaps(); - runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-version", release!]); + runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]); await runReleaseValidation(release!); log("rees_posthog_sourcemap_upload_complete", { release }); return 0; diff --git a/review-enrichment/test/posthog-upload.test.ts b/review-enrichment/test/posthog-upload.test.ts index 4fb07d91e..644401490 100644 --- a/review-enrichment/test/posthog-upload.test.ts +++ b/review-enrichment/test/posthog-upload.test.ts @@ -100,7 +100,7 @@ test("upload-sourcemaps skips the PostHog upload and exits 0 when required confi assert.throws(() => readFileSync(logPath, "utf8")); }); -test("upload-sourcemaps calls posthog-cli inject then upload with an explicit --release-version", async () => { +test("upload-sourcemaps calls posthog-cli inject then upload with explicit --release-name/--release-version", async () => { const { cliPath, logPath } = postHogCliStub(); const result = await runUploadSourcemaps({ @@ -118,11 +118,34 @@ test("upload-sourcemaps calls posthog-cli inject then upload with an explicit -- .split("\n") .map((line) => JSON.parse(line) as string[]); - assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-version", "loopover-rees@abc123"]); - assert.deepEqual(calls[1], ["sourcemap", "upload", "--directory", "dist", "--release-version", "loopover-rees@abc123"]); + // Split at the first "@", not a bare --release-version: passing the combined string as --release-version + // alone would leave --release-name unset, and posthog-cli auto-derives its own from git/package.json + // instead, silently doubling the stored release id (see splitRelease's own comment in the source). + assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]); + assert.deepEqual(calls[1], ["sourcemap", "upload", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]); assert.match(result.stdout, /rees_posthog_sourcemap_upload_complete/); }); +test("upload-sourcemaps falls back to a default release-name when POSTHOG_RELEASE has no '@'", async () => { + const { cliPath, logPath } = postHogCliStub(); + + const result = await runUploadSourcemaps({ + ...process.env, + POSTHOG_CLI_PATH: cliPath, + POSTHOG_CLI_API_KEY: "phx_test", + POSTHOG_CLI_PROJECT_ID: "42", + POSTHOG_RELEASE: "abc123", + REES_POSTHOG_VALIDATE_RELEASE: "0", + }); + + assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`); + const calls = readFileSync(logPath, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line) as string[]); + assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]); +}); + test("upload-sourcemaps logs posthog-cli's own stdout/stderr when it writes any", async () => { const { cliPath, logPath } = postHogCliStub({ echoOutput: "posthog-cli: uploaded 3 sourcemaps" }); diff --git a/scripts/deploy-selfhost-prebuilt.sh b/scripts/deploy-selfhost-prebuilt.sh index c36a71b60..203da92da 100755 --- a/scripts/deploy-selfhost-prebuilt.sh +++ b/scripts/deploy-selfhost-prebuilt.sh @@ -43,7 +43,7 @@ run_node_build() { } run_posthog_upload() { - local api_key project_id host uid gid + local api_key project_id host uid gid release_name release_version api_key="${POSTHOG_CLI_API_KEY:-$(env_get POSTHOG_CLI_API_KEY || true)}" project_id="${POSTHOG_CLI_PROJECT_ID:-$(env_get POSTHOG_CLI_PROJECT_ID || true)}" @@ -61,12 +61,19 @@ run_posthog_upload() { uid="$(id -u)" gid="$(id -g)" + # posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags, + # combined server-side into "{name}@{version}". Passing our already-combined POSTHOG_RELEASE as + # --release-version alone leaves --release-name unset, so the CLI auto-derives one from git/package.json + # instead -- silently doubling the stored release id and breaking "Validate PostHog release"-style lookups. + release_name="${POSTHOG_RELEASE%%@*}" + release_version="${POSTHOG_RELEASE#*@}" echo "selfhost deploy: injecting and uploading PostHog source maps for $POSTHOG_RELEASE" docker run --rm \ -e HOME=/tmp \ -e npm_config_cache=/tmp/.npm \ - -e POSTHOG_RELEASE \ + -e POSTHOG_RELEASE_NAME="$release_name" \ + -e POSTHOG_RELEASE_VERSION="$release_version" \ -e POSTHOG_CLI_API_KEY="$api_key" \ -e POSTHOG_CLI_PROJECT_ID="$project_id" \ ${host:+-e POSTHOG_CLI_HOST="$host"} \ @@ -76,7 +83,7 @@ run_posthog_upload() { -v "$PWD:/work" \ -w /work \ "$NODE_IMAGE" \ - sh -lc 'apt-get update >/dev/null && apt-get install -y --no-install-recommends ca-certificates >/dev/null && npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-version "$POSTHOG_RELEASE" && node --experimental-strip-types scripts/validate-selfhost-sourcemap.ts && npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-version "$POSTHOG_RELEASE" && chown -R "$HOST_UID:$HOST_GID" dist node_modules package-lock.json' + sh -lc 'apt-get update >/dev/null && apt-get install -y --no-install-recommends ca-certificates >/dev/null && npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION" && node --experimental-strip-types scripts/validate-selfhost-sourcemap.ts && npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION" && chown -R "$HOST_UID:$HOST_GID" dist node_modules package-lock.json' } run_init_secrets() { diff --git a/src/selfhost/posthog.ts b/src/selfhost/posthog.ts index 3683f60ae..77b0c946b 100644 --- a/src/selfhost/posthog.ts +++ b/src/selfhost/posthog.ts @@ -42,6 +42,7 @@ type PostHogEventMessage = import("posthog-node").EventMessage; let client: PostHogClient | undefined; let active = false; let posthogEnvironment = "production"; +let activeRelease: string | undefined; /** No per-user identity is tracked by this sink (operational error events, not user analytics) -- every event * shares one anonymous, constant distinct id, mirroring src/mcp/telemetry.ts's identical MCP_TELEMETRY_DISTINCT_ID @@ -136,6 +137,7 @@ export async function initPostHog(env: NodeJS.ProcessEnv): Promise { await loadNodeHasher(); const { PostHog } = await import("posthog-node"); posthogEnvironment = processEnvString(env, "POSTHOG_ENVIRONMENT") ?? "production"; + activeRelease = resolvePostHogRelease(env); const host = processEnvString(env, "POSTHOG_HOST") ?? DEFAULT_POSTHOG_HOST; client = new PostHog(apiKey, { host, @@ -175,6 +177,7 @@ export function capturePostHogError(error: unknown, context?: Record } }).process?.env?.POSTHOG_SERVER_NAME) ?? hostname(); properties.environment = posthogEnvironment; + if (activeRelease) properties.release = activeRelease; client.captureException(namedCaptureError(error, eventName), POSTHOG_DISTINCT_ID, properties); } @@ -186,6 +189,7 @@ export function capturePostHogReviewFailure(error: unknown, context?: Record { await run(); expect(process.exitCode).toBe(0); const posthogCalls = spawnSyncMock.mock.calls.filter(([command]) => isPostHogCliCall(command)).map(([, args]) => args); + // Split at the first "@", not a bare --release-version: passing the combined string as --release-version + // alone would leave --release-name unset, and posthog-cli auto-derives its own from git/package.json + // instead, silently doubling the stored release id (see splitRelease's own comment in the source). expect(posthogCalls).toEqual([ - ["sourcemap", "inject", "--directory", "dist", "--release-version", "loopover-discovery-index@abc123"], - ["sourcemap", "upload", "--directory", "dist", "--release-version", "loopover-discovery-index@abc123"], + ["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"], + ["sourcemap", "upload", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"], ]); expect(console.log).toHaveBeenCalledWith(expect.stringContaining("discovery_index_posthog_sourcemap_upload_complete")); }); + it("falls back to a default release-name when POSTHOG_RELEASE has no '@'", async () => { + setEnv({ POSTHOG_RELEASE: "abc123" }); + await run(); + expect(process.exitCode).toBe(0); + const posthogCalls = spawnSyncMock.mock.calls.filter(([command]) => isPostHogCliCall(command)).map(([, args]) => args); + expect(posthogCalls[0]).toEqual(["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"]); + }); + it("treats a non-strict upload failure as a soft failure (exit 0) with the reason logged", async () => { spawnSyncMock.mockImplementation((command: string, args: string[]) => { if (isPostHogCliCall(command) && args[1] === "upload") return { status: 1, stdout: "", stderr: "upload rejected" }; diff --git a/test/unit/selfhost-posthog-release.test.ts b/test/unit/selfhost-posthog-release.test.ts index 976d7a42a..e20ceb997 100644 --- a/test/unit/selfhost-posthog-release.test.ts +++ b/test/unit/selfhost-posthog-release.test.ts @@ -6,8 +6,18 @@ const read = (path: string) => readFileSync(path, "utf8"); describe("self-host PostHog release wiring", () => { it("keeps source-map uploads in the maintainer release workflow only", () => { const releaseWorkflow = read(".github/workflows/release-selfhost.yml"); - expect(releaseWorkflow).toContain("sourcemap inject --directory dist --release-version"); - expect(releaseWorkflow).toContain("sourcemap upload --directory dist --release-version"); + // Explicit --release-name AND --release-version, never a bare --release-version: posthog-cli + // auto-derives its own release-name from git/package.json when --release-name is omitted (this repo + // resolves to "loopover"), silently doubling the stored release id into + // "loopover@loopover-orb@$VERSION" -- which the "Validate PostHog release" step below would never find. + expect(releaseWorkflow).toContain( + 'sourcemap inject --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"', + ); + expect(releaseWorkflow).toContain( + 'sourcemap upload --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"', + ); + expect(releaseWorkflow).toContain("POSTHOG_RELEASE_NAME: loopover-orb"); + expect(releaseWorkflow).toContain("POSTHOG_RELEASE_VERSION: ${{ steps.version.outputs.v }}"); // No separate "create release"/"set-commits"/"finalize" steps -- PostHog release metadata is a // byproduct of the inject/upload calls themselves, unlike Sentry's releases/commits/deploys/finalize // lifecycle this replaced. diff --git a/test/unit/selfhost-posthog.test.ts b/test/unit/selfhost-posthog.test.ts index 2b1b34161..9a1cb5e0b 100644 --- a/test/unit/selfhost-posthog.test.ts +++ b/test/unit/selfhost-posthog.test.ts @@ -222,6 +222,18 @@ describe("capturePostHogError", () => { delete process.env.POSTHOG_REPO_MIN_SEVERITY; } }); + + it("attaches the resolved release when POSTHOG_RELEASE is configured", async () => { + await initPostHog({ POSTHOG_API_KEY: "phc_test_key", POSTHOG_RELEASE: "loopover-orb@1.2.3" } as unknown as NodeJS.ProcessEnv); + capturePostHogError(new Error("boom")); + expect(lastCapturedProperties().release).toBe("loopover-orb@1.2.3"); + }); + + it("omits release when neither POSTHOG_RELEASE nor LOOPOVER_VERSION is set", async () => { + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + capturePostHogError(new Error("boom")); + expect(lastCapturedProperties().release).toBeUndefined(); + }); }); describe("capturePostHogReviewFailure", () => { @@ -237,6 +249,12 @@ describe("capturePostHogReviewFailure", () => { expect(lastCapturedProperties().kind).toBe("review_failure"); }); + it("attaches the resolved release when configured", async () => { + await initPostHog({ POSTHOG_API_KEY: "phc_test_key", LOOPOVER_VERSION: "9.9.9" } as unknown as NodeJS.ProcessEnv); + capturePostHogReviewFailure(new Error("review failed")); + expect(lastCapturedProperties().release).toBe("9.9.9"); + }); + it("respects POSTHOG_MIN_SEVERITY -- suppressed above error", async () => { process.env.POSTHOG_MIN_SEVERITY = "critical"; try { @@ -370,6 +388,12 @@ describe("forwardStructuredLogToPostHog", () => { forwardStructuredLogToPostHog(JSON.stringify({ level: "error", repository: "owner/repo" })); expect(lastCapturedException().message).toContain("(owner/repo)"); }); + + it("attaches the resolved release when configured", async () => { + await initPostHog({ POSTHOG_API_KEY: "phc_test_key", POSTHOG_RELEASE: "loopover-orb@1.2.3" } as unknown as NodeJS.ProcessEnv); + forwardStructuredLogToPostHog(JSON.stringify({ level: "error", event: "x" })); + expect(lastCapturedProperties().release).toBe("loopover-orb@1.2.3"); + }); }); describe("installPostHogStructuredLogForwarding", () => {