[codex] Add actor-scoped tracking payloads#2330
Open
WcaleNieWolny wants to merge 3 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR introduces tracking payload v2 by shifting telemetry to organization-scoped identity ( ChangesTracking Payload v2 with Organization-Scoped Identity
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11c2023830
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+145
to
+147
| const trackedBody = trackingV2 | ||
| ? { ...trackOptions, user_id: trackingUserId, tags: trackedTags } | ||
| : requestedUserId |
There was a problem hiding this comment.
For tracking_version: 2 requests, this branch rewrites user_id to the authenticated actor ID, but the existing onboarding path later treats trackedBody.user_id as an organization ID (orgs.id) when event === 'onboarding-step-done'. As a result, a valid v2 onboarding event (with org_id set) will look up the org using a user UUID, fail the .single() query, and return error_fetching_organization_or_app instead of tracking/sending Bento. This breaks the onboarding-done telemetry flow for v2 clients.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/build/onboarding/telemetry.ts (1)
82-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMigrate
trackBuilderOnboardingActionto v2 payload fields as well.
trackBuilderOnboardingStepnow usesorg_idandtracking_version: 2, buttrackBuilderOnboardingActionstill sendsuser_id: input.orgId. This keeps org IDs overloaded intouser_idand leaves onboarding action events on the old contract.Suggested fix
try { await sendEvent(input.apikey, { event: 'Builder Onboarding Action', channel: 'builder-onboarding', icon: '🧭', notify: false, - user_id: input.orgId, + org_id: input.orgId, + tracking_version: 2, tags, }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/build/onboarding/telemetry.ts` around lines 82 - 88, The onboarding action telemetry is still using the old payload field user_id and lacks the v2 marker; update trackBuilderOnboardingAction to mirror trackBuilderOnboardingStep by renaming user_id: input.orgId to org_id: input.orgId and add tracking_version: 2 to the payload, keeping the rest of the fields (event, channel, icon, notify, tags) unchanged so the event uses the v2 contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/settings/organization/Usage.vue`:
- Around line 46-47: The tracking event currently sets tracking_version: 2 while
org_id can be undefined (currentOrganization.value?.gid); update the code around
the sendEvent call (the call site that uses currentOrganization and
tracking_version) to only emit v2 events when a valid organization exists —
e.g., check that currentOrganization.value?.gid is truthy before calling
sendEvent or conditionally set tracking_version to 1/omit it when org_id is
missing; locate the sendEvent invocation that references currentOrganization and
adjust it to guard on currentOrganization.value?.gid (or branch to a fallback)
so v2 events are only sent with a valid org_id.
In `@supabase/functions/_backend/private/events.ts`:
- Around line 145-149: The trackingV2 branch incorrectly sets
trackedBody.user_id to the authenticated actor (trackingUserId) and later
onboarding enrichment treats that as an org ID; change trackedBody construction
so that when trackingV2 is true you include the authenticated actor as user_id
(trackingUserId) but also include the verified organization id under a separate
key (e.g., org_id: verifiedOrgId) so onboarding-step-done lookup uses org_id
instead of user_id; update any onboarding enrichment code that reads
trackedBody.user_id to read trackedBody.org_id for org lookups; adjust the
trackedBody creation in the branch that uses trackingV2 (and keep the existing
branches for requestedUserId/trackOptions) to include this org_id field.
In `@tests/events.test.ts`:
- Around line 54-78: The test labeled "tracks v2 events with actor user and
organization context" (and the other v2 event test in the same file) are using
it(...) but must use parallel tests; change the test declarations from it(...)
to it.concurrent(...) so both v2 event specs run in parallel; update the two
occurrences in tests/events.test.ts where the test descriptions mention "v2" or
"tracking_version: 2" to use it.concurrent instead of it.
---
Outside diff comments:
In `@cli/src/build/onboarding/telemetry.ts`:
- Around line 82-88: The onboarding action telemetry is still using the old
payload field user_id and lacks the v2 marker; update
trackBuilderOnboardingAction to mirror trackBuilderOnboardingStep by renaming
user_id: input.orgId to org_id: input.orgId and add tracking_version: 2 to the
payload, keeping the rest of the fields (event, channel, icon, notify, tags)
unchanged so the event uses the v2 contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d50b6188-8fdf-444f-aa2d-dded68c44657
📒 Files selected for processing (29)
cli/src/ai/telemetry.tscli/src/build/credentials-command.tscli/src/build/onboarding/telemetry.tscli/src/build/request.tscli/src/build/telemetry.tscli/src/utils.tssrc/components/dashboard/DemoOnboardingModal.vuesrc/components/dashboard/InviteTeammateModal.vuesrc/components/dashboard/StepsApp.vuesrc/components/dashboard/StepsBuild.vuesrc/components/dashboard/StepsBundle.vuesrc/components/dashboard/TrialBanner.vuesrc/pages/settings/organization/Plans.vuesrc/pages/settings/organization/Usage.vuesrc/services/tracking.tssupabase/functions/_backend/private/events.tssupabase/functions/_backend/public/build/ai_analyze.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/start.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_tracking.tstests/ai-analysis-telemetry.unit.test.tstests/build-ai-analyze.test.tstests/build-lifecycle-emit.unit.test.tstests/build-start-log-token.test.tstests/builder-onboarding-telemetry.unit.test.tstests/builder-upload-telemetry.unit.test.tstests/events.test.ts
Comment on lines
+145
to
+149
| const trackedBody = trackingV2 | ||
| ? { ...trackOptions, user_id: trackingUserId, tags: trackedTags } | ||
| : requestedUserId | ||
| ? { ...trackOptions, user_id: trackingUserId } | ||
| : trackOptions |
Contributor
There was a problem hiding this comment.
Use verified org ID (not actor ID) for onboarding org lookup in tracking v2
In tracking v2, user_id is the authenticated actor. The onboarding enrichment path still treats trackedBody.user_id as an org ID, which can fail org lookup and abort onboarding-step-done event processing.
Suggested fix
+ const onboardingOrgId = trackingV2 ? verifiedOrgId : trackedBody.user_id
- if (trackedBody.user_id && appId && trackedBody.event === 'onboarding-step-done') {
+ if (onboardingOrgId && appId && trackedBody.event === 'onboarding-step-done') {
onboardingBentoEvent = await Promise.all([
supabase
.from('orgs')
.select('*')
- .eq('id', trackedBody.user_id)
+ .eq('id', onboardingOrgId)
.single(),
supabase
.from('apps')
.select('*')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/private/events.ts` around lines 145 - 149, The
trackingV2 branch incorrectly sets trackedBody.user_id to the authenticated
actor (trackingUserId) and later onboarding enrichment treats that as an org ID;
change trackedBody construction so that when trackingV2 is true you include the
authenticated actor as user_id (trackingUserId) but also include the verified
organization id under a separate key (e.g., org_id: verifiedOrgId) so
onboarding-step-done lookup uses org_id instead of user_id; update any
onboarding enrichment code that reads trackedBody.user_id to read
trackedBody.org_id for org lookups; adjust the trackedBody creation in the
branch that uses trackingV2 (and keep the existing branches for
requestedUserId/trackOptions) to include this org_id field.
Comment on lines
+54
to
+78
| it('tracks v2 events with actor user and organization context', async () => { | ||
| const response = await fetch(`${BASE_URL}/private/events`, { | ||
| method: 'POST', | ||
| headers: { | ||
| capgkey: headers.Authorization, | ||
| }, | ||
| body: JSON.stringify({ | ||
| channel: 'test', | ||
| event: 'test_event_v2', | ||
| description: 'Testing v2 event tracking', | ||
| icon: '🧪', | ||
| notify: false, | ||
| org_id: ORG_ID, | ||
| tracking_version: 2, | ||
| tags: { | ||
| app_id: APPNAME_EVENT, | ||
| test: true, | ||
| }, | ||
| }), | ||
| }) | ||
|
|
||
| const data = await response.json() as { status: string } | ||
| expect(response.status).toBe(200) | ||
| expect(data.status).toBe('ok') | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "^\s*it\(" tests/events.test.tsRepository: Cap-go/capgo
Length of output: 824
Use it.concurrent for the two v2 event tests
In tests/events.test.ts, these cases are declared with it(...) but the repo requires it.concurrent() in tests/**/*.test.ts for parallel execution:
Suggested change
- it('tracks v2 events with actor user and organization context', async () => {
+ it.concurrent('tracks v2 events with actor user and organization context', async () => {
@@
- it('rejects v2 events for foreign organizations', async () => {
+ it.concurrent('rejects v2 events for foreign organizations', async () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/events.test.ts` around lines 54 - 78, The test labeled "tracks v2
events with actor user and organization context" (and the other v2 event test in
the same file) are using it(...) but must use parallel tests; change the test
declarations from it(...) to it.concurrent(...) so both v2 event specs run in
parallel; update the two occurrences in tests/events.test.ts where the test
descriptions mention "v2" or "tracking_version: 2" to use it.concurrent instead
of it.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (AI generated)
tracking_version: 2+org_idsupport to/private/eventsso new callers track the authenticated actor while preserving organization grouping.user_id.requested_byfor delayed builder lifecycle and AI analysis events while keepingowner_orgas the PostHog organization group.Motivation (AI generated)
Capgo analytics currently mix person IDs and organization IDs in
user_id, which makes it hard to answer whether the same human used the dashboard, CLI onboarding, and builder. A versioned payload keeps old clients compatible while allowing new CLI/frontend events to carry both actor and org context cleanly.Business Impact (AI generated)
This improves funnel attribution across dashboard onboarding, CLI onboarding, builder uploads, build lifecycle, and AI build analysis. Better cross-product attribution should make onboarding drop-off and builder adoption easier to diagnose without breaking existing tracking clients.
Test Plan (AI generated)
bun lintbun lint:backendbunx vitest run tests/ai-analysis-telemetry.unit.test.ts tests/builder-onboarding-telemetry.unit.test.ts tests/builder-upload-telemetry.unit.test.ts tests/build-lifecycle-emit.unit.test.ts tests/build-ai-analyze.test.ts tests/build-start-log-token.test.tsbun typecheckbun run supabase:with-env -- bunx vitest run tests/events.test.tscould not be validated locally because this worktree has no Supabase DB container; all existing and new cases returned 503.Generated with AI
Summary by CodeRabbit