Skip to content

Commit cdad5c2

Browse files
JSONboredJSONbored
authored
fix(posthog): pass release-name and release-version as separate CLI flags (#8624)
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.
1 parent a9d9c0e commit cdad5c2

9 files changed

Lines changed: 130 additions & 17 deletions

File tree

‎.github/workflows/release-selfhost.yml‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,13 @@ jobs:
161161
POSTHOG_CLI_API_KEY: ${{ secrets.POSTHOG_CLI_API_KEY }}
162162
POSTHOG_CLI_PROJECT_ID: ${{ secrets.POSTHOG_CLI_PROJECT_ID }}
163163
POSTHOG_CLI_HOST: ${{ secrets.POSTHOG_CLI_HOST }}
164-
POSTHOG_RELEASE: ${{ steps.version.outputs.release }}
164+
# Passed as separate --release-name/--release-version flags below rather than one combined
165+
# --release-version string: posthog-cli otherwise auto-derives its own release-name from git/
166+
# package.json (this repo resolves to "loopover") and prepends it, so an unqualified
167+
# --release-version "loopover-orb@$VERSION" silently becomes the stored release
168+
# "loopover@loopover-orb@$VERSION" -- never matching what "Validate PostHog release" below looks up.
169+
POSTHOG_RELEASE_NAME: loopover-orb
170+
POSTHOG_RELEASE_VERSION: ${{ steps.version.outputs.v }}
165171
run: |
166172
set -euo pipefail
167173
test -n "$POSTHOG_CLI_API_KEY"
@@ -172,9 +178,9 @@ jobs:
172178
if [ -z "${POSTHOG_CLI_HOST:-}" ]; then unset POSTHOG_CLI_HOST; fi
173179
# No separate "create release" step -- PostHog release metadata is a byproduct of the inject/
174180
# upload calls below, unlike Sentry's releases/commits/deploys/finalize lifecycle this replaces.
175-
npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-version "$POSTHOG_RELEASE"
181+
npx -y "$POSTHOG_CLI_PACKAGE" sourcemap inject --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"
176182
node --experimental-strip-types scripts/validate-selfhost-sourcemap.ts
177-
npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-version "$POSTHOG_RELEASE"
183+
npx -y "$POSTHOG_CLI_PACKAGE" sourcemap upload --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"
178184
179185
- uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4
180186
- uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4

‎packages/discovery-index/src/upload-sourcemaps.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ function nonBlank(value: string | undefined): string | undefined {
2525
return text ? text : undefined;
2626
}
2727

28+
// posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags, which
29+
// it combines server-side into the release id "{name}@{version}". Passing our own already-combined
30+
// POSTHOG_RELEASE (e.g. "loopover-discovery-index@<sha>") as --release-version alone leaves --release-name
31+
// unset, and the CLI then auto-derives one from git/package.json instead -- silently doubling up into
32+
// "<auto-derived>@loopover-discovery-index@<sha>", which never matches what runReleaseValidation looks up.
33+
// Splitting our own convention at its first "@" reproduces exactly the release id we already expect.
34+
function splitRelease(release: string, defaultName: string): { name: string; version: string } {
35+
const at = release.indexOf("@");
36+
if (at === -1) return { name: defaultName, version: release };
37+
return { name: release.slice(0, at), version: release.slice(at + 1) };
38+
}
39+
2840
function log(event: string, fields: Record<string, unknown> = {}): void {
2941
console.log(JSON.stringify({ event, ...fields }));
3042
}
@@ -161,9 +173,10 @@ async function main(): Promise<number> {
161173
// calls below. Explicit --release-version rather than posthog-cli's own git-metadata auto-detection: the
162174
// Dockerfile's build stage only copies packages/loopover-engine and packages/discovery-index source,
163175
// never `.git`, so auto-detection has nothing to inspect at container-startup time.
164-
runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-version", release!]);
176+
const { name: releaseName, version: releaseVersion } = splitRelease(release!, "loopover-discovery-index");
177+
runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]);
165178
validateSourceMaps();
166-
runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-version", release!]);
179+
runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]);
167180
await runReleaseValidation(release!);
168181
log("discovery_index_posthog_sourcemap_upload_complete", { release });
169182
return 0;

‎review-enrichment/src/upload-sourcemaps.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ function nonBlank(value: string | undefined): string | undefined {
2828
return text ? text : undefined;
2929
}
3030

31+
// posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags, which
32+
// it combines server-side into the release id "{name}@{version}". Passing our own already-combined
33+
// POSTHOG_RELEASE (e.g. "loopover-rees@<sha>") as --release-version alone leaves --release-name unset, and
34+
// the CLI then auto-derives one from git/package.json instead -- silently doubling up into
35+
// "<auto-derived>@loopover-rees@<sha>", which never matches what runReleaseValidation looks up. Splitting
36+
// our own convention at its first "@" reproduces exactly the release id we already expect.
37+
function splitRelease(release: string, defaultName: string): { name: string; version: string } {
38+
const at = release.indexOf("@");
39+
if (at === -1) return { name: defaultName, version: release };
40+
return { name: release.slice(0, at), version: release.slice(at + 1) };
41+
}
42+
3143
function log(event: string, fields: Record<string, unknown> = {}): void {
3244
console.log(JSON.stringify({ event, ...fields }));
3345
}
@@ -166,9 +178,10 @@ async function main(): Promise<number> {
166178
// No separate "create release" step (PostHog release metadata is a byproduct of inject/upload) and an
167179
// explicit --release-version rather than posthog-cli's own git-metadata auto-detection -- the deploy
168180
// environment isn't guaranteed to have a usable .git checkout at this step.
169-
runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-version", release!]);
181+
const { name: releaseName, version: releaseVersion } = splitRelease(release!, "loopover-rees");
182+
runPostHog(["sourcemap", "inject", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]);
170183
validateSourceMaps();
171-
runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-version", release!]);
184+
runPostHog(["sourcemap", "upload", "--directory", "dist", "--release-name", releaseName, "--release-version", releaseVersion]);
172185
await runReleaseValidation(release!);
173186
log("rees_posthog_sourcemap_upload_complete", { release });
174187
return 0;

‎review-enrichment/test/posthog-upload.test.ts‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ test("upload-sourcemaps skips the PostHog upload and exits 0 when required confi
100100
assert.throws(() => readFileSync(logPath, "utf8"));
101101
});
102102

103-
test("upload-sourcemaps calls posthog-cli inject then upload with an explicit --release-version", async () => {
103+
test("upload-sourcemaps calls posthog-cli inject then upload with explicit --release-name/--release-version", async () => {
104104
const { cliPath, logPath } = postHogCliStub();
105105

106106
const result = await runUploadSourcemaps({
@@ -118,11 +118,34 @@ test("upload-sourcemaps calls posthog-cli inject then upload with an explicit --
118118
.split("\n")
119119
.map((line) => JSON.parse(line) as string[]);
120120

121-
assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-version", "loopover-rees@abc123"]);
122-
assert.deepEqual(calls[1], ["sourcemap", "upload", "--directory", "dist", "--release-version", "loopover-rees@abc123"]);
121+
// Split at the first "@", not a bare --release-version: passing the combined string as --release-version
122+
// alone would leave --release-name unset, and posthog-cli auto-derives its own from git/package.json
123+
// instead, silently doubling the stored release id (see splitRelease's own comment in the source).
124+
assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]);
125+
assert.deepEqual(calls[1], ["sourcemap", "upload", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]);
123126
assert.match(result.stdout, /rees_posthog_sourcemap_upload_complete/);
124127
});
125128

129+
test("upload-sourcemaps falls back to a default release-name when POSTHOG_RELEASE has no '@'", async () => {
130+
const { cliPath, logPath } = postHogCliStub();
131+
132+
const result = await runUploadSourcemaps({
133+
...process.env,
134+
POSTHOG_CLI_PATH: cliPath,
135+
POSTHOG_CLI_API_KEY: "phx_test",
136+
POSTHOG_CLI_PROJECT_ID: "42",
137+
POSTHOG_RELEASE: "abc123",
138+
REES_POSTHOG_VALIDATE_RELEASE: "0",
139+
});
140+
141+
assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`);
142+
const calls = readFileSync(logPath, "utf8")
143+
.trim()
144+
.split("\n")
145+
.map((line) => JSON.parse(line) as string[]);
146+
assert.deepEqual(calls[0], ["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-rees", "--release-version", "abc123"]);
147+
});
148+
126149
test("upload-sourcemaps logs posthog-cli's own stdout/stderr when it writes any", async () => {
127150
const { cliPath, logPath } = postHogCliStub({ echoOutput: "posthog-cli: uploaded 3 sourcemaps" });
128151

‎scripts/deploy-selfhost-prebuilt.sh‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ run_node_build() {
4343
}
4444

4545
run_posthog_upload() {
46-
local api_key project_id host uid gid
46+
local api_key project_id host uid gid release_name release_version
4747

4848
api_key="${POSTHOG_CLI_API_KEY:-$(env_get POSTHOG_CLI_API_KEY || true)}"
4949
project_id="${POSTHOG_CLI_PROJECT_ID:-$(env_get POSTHOG_CLI_PROJECT_ID || true)}"
@@ -61,12 +61,19 @@ run_posthog_upload() {
6161

6262
uid="$(id -u)"
6363
gid="$(id -g)"
64+
# posthog-cli's sourcemap inject/upload take --release-name and --release-version as SEPARATE flags,
65+
# combined server-side into "{name}@{version}". Passing our already-combined POSTHOG_RELEASE as
66+
# --release-version alone leaves --release-name unset, so the CLI auto-derives one from git/package.json
67+
# instead -- silently doubling the stored release id and breaking "Validate PostHog release"-style lookups.
68+
release_name="${POSTHOG_RELEASE%%@*}"
69+
release_version="${POSTHOG_RELEASE#*@}"
6470

6571
echo "selfhost deploy: injecting and uploading PostHog source maps for $POSTHOG_RELEASE"
6672
docker run --rm \
6773
-e HOME=/tmp \
6874
-e npm_config_cache=/tmp/.npm \
69-
-e POSTHOG_RELEASE \
75+
-e POSTHOG_RELEASE_NAME="$release_name" \
76+
-e POSTHOG_RELEASE_VERSION="$release_version" \
7077
-e POSTHOG_CLI_API_KEY="$api_key" \
7178
-e POSTHOG_CLI_PROJECT_ID="$project_id" \
7279
${host:+-e POSTHOG_CLI_HOST="$host"} \
@@ -76,7 +83,7 @@ run_posthog_upload() {
7683
-v "$PWD:/work" \
7784
-w /work \
7885
"$NODE_IMAGE" \
79-
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'
86+
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'
8087
}
8188

8289
run_init_secrets() {

‎src/selfhost/posthog.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type PostHogEventMessage = import("posthog-node").EventMessage;
4343
let client: PostHogClient | undefined;
4444
let active = false;
4545
let posthogEnvironment = "production";
46+
let activeRelease: string | undefined;
4647

4748
/** No per-user identity is tracked by this sink (operational error events, not user analytics) -- every event
4849
* shares one anonymous, constant distinct id, mirroring src/mcp/telemetry.ts's identical MCP_TELEMETRY_DISTINCT_ID
@@ -137,6 +138,7 @@ export async function initPostHog(env: NodeJS.ProcessEnv): Promise<boolean> {
137138
await loadNodeHasher();
138139
const { PostHog } = await import("posthog-node");
139140
posthogEnvironment = processEnvString(env, "POSTHOG_ENVIRONMENT") ?? "production";
141+
activeRelease = resolvePostHogRelease(env);
140142
const host = processEnvString(env, "POSTHOG_HOST") ?? DEFAULT_POSTHOG_HOST;
141143
client = new PostHog(apiKey, {
142144
host,
@@ -176,6 +178,7 @@ export function capturePostHogError(error: unknown, context?: Record<string, unk
176178
const properties = operationalProperties(context);
177179
properties.server_name = nonBlank((globalThis as unknown as { process?: { env?: Record<string, string | undefined> } }).process?.env?.POSTHOG_SERVER_NAME) ?? hostname();
178180
properties.environment = posthogEnvironment;
181+
if (activeRelease) properties.release = activeRelease;
179182
client.captureException(namedCaptureError(error, eventName), POSTHOG_DISTINCT_ID, properties);
180183
}
181184

@@ -187,6 +190,7 @@ export function capturePostHogReviewFailure(error: unknown, context?: Record<str
187190
if (!meetsSeverityThreshold("error", resolvePostHogMinSeverity(contextRepoFullName(context)))) return;
188191
const properties = operationalProperties(context);
189192
properties.kind = "review_failure";
193+
if (activeRelease) properties.release = activeRelease;
190194
client.captureException(namedCaptureError(error, eventName), POSTHOG_DISTINCT_ID, properties);
191195
}
192196

@@ -252,6 +256,7 @@ export function forwardStructuredLogToPostHog(line: unknown, fromErrorSink = fal
252256
properties.severity = loopoverSeverity;
253257
if (event) properties.event_slug = event;
254258
if (subEvent) properties.event_sub_slug = subEvent;
259+
if (activeRelease) properties.release = activeRelease;
255260
client.captureException(errorEvent, POSTHOG_DISTINCT_ID, properties);
256261
}
257262

@@ -404,5 +409,6 @@ export function resetPostHogForTest(): void {
404409
client = undefined;
405410
active = false;
406411
posthogEnvironment = "production";
412+
activeRelease = undefined;
407413
resetRedactionScrubForTest();
408414
}

‎test/unit/discovery-index/upload-sourcemaps.test.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,24 @@ describe("discovery-index upload-sourcemaps -- PostHog (#8289)", () => {
136136
await run();
137137
expect(process.exitCode).toBe(0);
138138
const posthogCalls = spawnSyncMock.mock.calls.filter(([command]) => isPostHogCliCall(command)).map(([, args]) => args);
139+
// Split at the first "@", not a bare --release-version: passing the combined string as --release-version
140+
// alone would leave --release-name unset, and posthog-cli auto-derives its own from git/package.json
141+
// instead, silently doubling the stored release id (see splitRelease's own comment in the source).
139142
expect(posthogCalls).toEqual([
140-
["sourcemap", "inject", "--directory", "dist", "--release-version", "loopover-discovery-index@abc123"],
141-
["sourcemap", "upload", "--directory", "dist", "--release-version", "loopover-discovery-index@abc123"],
143+
["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"],
144+
["sourcemap", "upload", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"],
142145
]);
143146
expect(console.log).toHaveBeenCalledWith(expect.stringContaining("discovery_index_posthog_sourcemap_upload_complete"));
144147
});
145148

149+
it("falls back to a default release-name when POSTHOG_RELEASE has no '@'", async () => {
150+
setEnv({ POSTHOG_RELEASE: "abc123" });
151+
await run();
152+
expect(process.exitCode).toBe(0);
153+
const posthogCalls = spawnSyncMock.mock.calls.filter(([command]) => isPostHogCliCall(command)).map(([, args]) => args);
154+
expect(posthogCalls[0]).toEqual(["sourcemap", "inject", "--directory", "dist", "--release-name", "loopover-discovery-index", "--release-version", "abc123"]);
155+
});
156+
146157
it("treats a non-strict upload failure as a soft failure (exit 0) with the reason logged", async () => {
147158
spawnSyncMock.mockImplementation((command: string, args: string[]) => {
148159
if (isPostHogCliCall(command) && args[1] === "upload") return { status: 1, stdout: "", stderr: "upload rejected" };

‎test/unit/selfhost-posthog-release.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,18 @@ const read = (path: string) => readFileSync(path, "utf8");
66
describe("self-host PostHog release wiring", () => {
77
it("keeps source-map uploads in the maintainer release workflow only", () => {
88
const releaseWorkflow = read(".github/workflows/release-selfhost.yml");
9-
expect(releaseWorkflow).toContain("sourcemap inject --directory dist --release-version");
10-
expect(releaseWorkflow).toContain("sourcemap upload --directory dist --release-version");
9+
// Explicit --release-name AND --release-version, never a bare --release-version: posthog-cli
10+
// auto-derives its own release-name from git/package.json when --release-name is omitted (this repo
11+
// resolves to "loopover"), silently doubling the stored release id into
12+
// "loopover@loopover-orb@$VERSION" -- which the "Validate PostHog release" step below would never find.
13+
expect(releaseWorkflow).toContain(
14+
'sourcemap inject --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"',
15+
);
16+
expect(releaseWorkflow).toContain(
17+
'sourcemap upload --directory dist --release-name "$POSTHOG_RELEASE_NAME" --release-version "$POSTHOG_RELEASE_VERSION"',
18+
);
19+
expect(releaseWorkflow).toContain("POSTHOG_RELEASE_NAME: loopover-orb");
20+
expect(releaseWorkflow).toContain("POSTHOG_RELEASE_VERSION: ${{ steps.version.outputs.v }}");
1121
// No separate "create release"/"set-commits"/"finalize" steps -- PostHog release metadata is a
1222
// byproduct of the inject/upload calls themselves, unlike Sentry's releases/commits/deploys/finalize
1323
// lifecycle this replaced.

‎test/unit/selfhost-posthog.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,18 @@ describe("capturePostHogError", () => {
223223
delete process.env.POSTHOG_REPO_MIN_SEVERITY;
224224
}
225225
});
226+
227+
it("attaches the resolved release when POSTHOG_RELEASE is configured", async () => {
228+
await initPostHog({ POSTHOG_API_KEY: "phc_test_key", POSTHOG_RELEASE: "loopover-orb@1.2.3" } as unknown as NodeJS.ProcessEnv);
229+
capturePostHogError(new Error("boom"));
230+
expect(lastCapturedProperties().release).toBe("loopover-orb@1.2.3");
231+
});
232+
233+
it("omits release when neither POSTHOG_RELEASE nor LOOPOVER_VERSION is set", async () => {
234+
await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv);
235+
capturePostHogError(new Error("boom"));
236+
expect(lastCapturedProperties().release).toBeUndefined();
237+
});
226238
});
227239

228240
describe("capturePostHogReviewFailure", () => {
@@ -238,6 +250,12 @@ describe("capturePostHogReviewFailure", () => {
238250
expect(lastCapturedProperties().kind).toBe("review_failure");
239251
});
240252

253+
it("attaches the resolved release when configured", async () => {
254+
await initPostHog({ POSTHOG_API_KEY: "phc_test_key", LOOPOVER_VERSION: "9.9.9" } as unknown as NodeJS.ProcessEnv);
255+
capturePostHogReviewFailure(new Error("review failed"));
256+
expect(lastCapturedProperties().release).toBe("9.9.9");
257+
});
258+
241259
it("respects POSTHOG_MIN_SEVERITY -- suppressed above error", async () => {
242260
process.env.POSTHOG_MIN_SEVERITY = "critical";
243261
try {
@@ -371,6 +389,12 @@ describe("forwardStructuredLogToPostHog", () => {
371389
forwardStructuredLogToPostHog(JSON.stringify({ level: "error", repository: "owner/repo" }));
372390
expect(lastCapturedException().message).toContain("(owner/repo)");
373391
});
392+
393+
it("attaches the resolved release when configured", async () => {
394+
await initPostHog({ POSTHOG_API_KEY: "phc_test_key", POSTHOG_RELEASE: "loopover-orb@1.2.3" } as unknown as NodeJS.ProcessEnv);
395+
forwardStructuredLogToPostHog(JSON.stringify({ level: "error", event: "x" }));
396+
expect(lastCapturedProperties().release).toBe("loopover-orb@1.2.3");
397+
});
374398
});
375399

376400
describe("installPostHogStructuredLogForwarding", () => {

0 commit comments

Comments
 (0)