feat(cli): default release mode to event - #91823
Conversation
|
😎 Stack merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. |
🦔 PostHog Review reviewed this pull requestFound 2 must fix, 4 should fix, 1 consider. Published 7 findings (view the review). |
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
Prompt To Fix All With AI### Issue 1
cli/src/sourcemaps/plain/upload.rs:74
**Standalone uploads lose releases**
When an existing workflow runs `sourcemap upload` without first applying event-mode injection, the new default uploads release-independent symbol sets without adding or validating `_posthogReleaseId`, causing symbolicated exceptions to lose their release attribution.
### Issue 2
cli/src/sourcemaps/args.rs:98-101
**Release-mode documentation remains stale**
The CLI now defaults these commands to `event`, but the published documentation still describes `symbol-set` as the default, directing users toward behavior that no longer matches the CLI and omitting the coordinates event mode requires.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(cli): correct the event mode release..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR updates the PostHog CLI’s error-tracking upload/inject flows to make --release-mode=event the default, so identical artifacts across releases no longer collide on the first uploader’s release attribution and instead resolve release per event.
Changes:
- Default
--release-modetoeventacross sourcemap inject/process/upload, Hermes clone/upload (and Hermes inject via shared args), and proguard upload. - Update CLI help text and warnings to reflect event mode semantics and the
symbol-setopt-out. - Add a shared “event mode needs full coordinates” predicate plus tests, and record the behavior change in a Sampo changeset.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cli/src/sourcemaps/plain/upload.rs | Switch default to event for plain sourcemap upload and update help text accordingly. |
| cli/src/sourcemaps/plain/mod.rs | Switch default to event for sourcemap process args and update help text. |
| cli/src/sourcemaps/inject.rs | Switch default to event for sourcemap inject and expand help text to cover web vs Hermes behavior. |
| cli/src/sourcemaps/hermes/upload.rs | Switch default to event, resolve Info.plist before validation, and use shared coordinate-completeness check. |
| cli/src/sourcemaps/hermes/mod.rs | Update tests to pin the new default and cover symbol-set opt-out, including hermes inject. |
| cli/src/sourcemaps/hermes/clone.rs | Switch default to event and update help text. |
| cli/src/sourcemaps/args.rs | Update ReleaseMode docs, refine conflict flag help text, add event_coordinates_complete, and add tests. |
| cli/src/proguard/upload.rs | Switch default to event, update warnings, and update tests for the new default and opt-out. |
| cli/.sampo/changesets/release-mode-event-default.md | Add release note for the default change and the updated warning behavior. |
Suppressed comments (2)
cli/src/proguard/upload.rs:76
- This warning says event-mode release resolution uses the app's "namespace and version", but the check requires a build number too (
event_coordinates_complete). Mentioning build here makes the warning consistent with the required coordinates and reduces confusion.
if *release_mode == ReleaseMode::Event && !resolved_release.event_coordinates_complete() {
warn!(
"--release-mode=event resolves each exception's release from the app's namespace and \
version. Pass --release-name, --release-version and --build matching the app's \
applicationId, versionName and versionCode, or exceptions will report no release."
cli/src/sourcemaps/hermes/upload.rs:76
- This warning says event-mode release resolution uses the app's "namespace and version", but the check requires a build number too (
event_coordinates_complete). Mentioning build here makes the warning consistent with the required coordinates and reduces confusion.
if *release_mode == ReleaseMode::Event && !release.event_coordinates_complete() {
warn!(
"--release-mode=event resolves each exception's release from the app's namespace and \
version. Pass --release-name, --release-version and --build matching the app's bundle \
identifier or applicationId, its version and its build number, or exceptions will \
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a153f12 to
7c56cf8
Compare
Uploaded symbol sets, source maps and mappings are now release-independent by default, and each exception resolves its own release. Pass --release-mode symbol-set to keep binding the release to the upload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
hermes upload resolved --info-plist after its event mode warning, so an iOS build was told its exceptions would report no release while the run created the correct one. proguard upload never checked --build, which the server packs into the release version it keys on. Both now share one predicate on ReleaseArgs. The inject help described the web release source only, though InjectArgs also backs hermes inject, which injects no release id. The conflict help claimed a rule that holds for sourcemap uploads and not for proguard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
The event variant said the release comes from an id injected into each chunk. That holds for a web build. Hermes and proguard inject nothing and resolve the release from the app version and namespace the SDK sends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
The stack's base PR deprecates the release mode on proguard upload, so this layer no longer flips its default. The changeset and the mode help drop their proguard mentions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177eam86deHk8HZYMKcJtbD
7c56cf8 to
ea78898
Compare
Deploy status
|
Related PRs
Event mode is live today, and it is opt-in. These PRs settle where it stays and where it goes.
Deprecating the mobile knobs. The symbol id on these paths is already a content hash, so two releases collide only when they ship a byte-identical artifact. An ordinary release that changes code gets its own symbol set and never collides. The dSYM path also lost release attribution for embedded targets, because one upload covers every target while it creates one release. Review asked for deprecation instead of removal, so every knob stays accepted as a warned no-op.
Making it the default.
React Native. Scopes the mode to the Hermes upload and defaults it to event. That is the one path where two releases really do ship the same artifact.
Problem
Changes
--release-modedefaults toeventon six commands.sourcemap inject,sourcemap process,sourcemap upload,hermes inject,hermes cloneandhermes upload.--release-mode symbol-setkeeps the old behavior. So doesPOSTHOG_RELEASE_MODE=symbol-set.proguard uploadis out of scope here. The base PR deprecates its release mode, so its mapping always binds.hermes uploadresolves--info-plistbefore it checks the release coordinates, so an iOS build that supplies them that way no longer gets a false warning.event_coordinates_completecheck requires name, version and build, because the server packs the build into the version it keys a release on.eventexperimental.How did you test this code?
symbol-setopt-out, acrosshermes inject,hermes cloneandhermes upload.event_mode_needs_every_release_coordinatecovers the shared predicate.Automatic notifications
The sampo changeset is
cli/.sampo/changesets/release-mode-event-default.md. It asks for a minor bump.Docs update
The posthog.com docs still call
symbol-setthe default. They need a separate PR.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with Claude Code (Opus 5); restacked and narrowed with Claude Code (Fable 5). Skills invoked:
/writing-pr-descriptions,/stacking-prs.The base PR of the former stack is merged and released, and this branch is rebased onto master, so the diff shows only this layer.
proguard upload --release-modestays a hidden no-op there, so no commit in history defaults proguard to event.