feat: batch envelope on POST /track, deprecate /track/batch + sync upstream/main#11
Conversation
This reverts commit c470b8b.
* init * update * performance changes * bump bklit * update bklit * bklit update
Openpanel-dev#386) * fix: honor inviteId on existing-user sign-in paths (Openpanel-dev#385) Invites were only consumed during new-account creation, so an invited person who authenticated as an existing user (OAuth sign-in or email sign-in) had their inviteId silently dropped. With zero org memberships they were redirected into the Create Project onboarding wizard instead of joining the inviting organization. - connectUserToOrganization is now idempotent: reuse an existing membership instead of creating a duplicate (no unique constraint on members.organizationId+userId), then still consume the invite. - OAuth handleExistingUser now accepts inviteId and connects the org (best-effort, logged) for both GitHub and Google callbacks. - signInEmail accepts an optional inviteId and consumes it on success; for 2FA users it is carried through the challenge via a short-lived cookie and consumed in signInTotp. - Frontend forwards inviteId on the sign-in path (email + OAuth), the login route reads it from search params, and the onboarding "Sign in" link preserves ?inviteId so returning users don't lose it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: prevent duplicate org memberships on invite consumption The findFirst + create sequence in connectUserToOrganization was non-atomic and could produce duplicate Member rows when an invite was consumed concurrently. Enforce uniqueness at the database level and make the membership creation atomic. - Add @@unique([organizationId, userId]) to the Member model. NULLs are distinct in Postgres, so pending email-only invite rows are unaffected. - Migration de-duplicates existing memberships (keeping the earliest) before creating the unique index so it is deploy-safe. - Replace the racy findFirst ?? create with an atomic upsert keyed on the composite unique, reusing an existing membership unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clients now send { "type": "batch", "payload": [event, ...] } to the
existing /track endpoint — the same contract upstream requested in PR Openpanel-dev#377,
so client integrations stay stable across the eventual upstream merge.
- /track dispatches type=batch through the same processBatch pipeline
as the legacy route (shared salts/geo, per-item validation, 202 with
accepted/rejected[])
- /track/batch kept as a deprecated alias for already-shipped clients;
marked deprecated in the OpenAPI schema
- body-hash dedup skipped for batch envelopes (offline-first SDKs retry
whole batches)
- batch tests parametrized to run against both transports; envelope-only
regression tests for single-event 200 and nested-batch rejection
- FORK-PATCHES.md documents the temporary divergences from upstream and
the sync playbook for when upstream's session rework lands
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- move the batch dedup-skip into duplicateHook next to the existing replay skip, dropping the router-level wrapper and its casts - share the batch events array schema between the canonical and legacy envelopes - single pass over batch results instead of two filters - document that the 10 MB body limit also applies to single-event bodies Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tch arms Adds batch coverage (both transports) for group/assign_group/increment/ decrement/replay dispatch, per-event __ip geo override, non-object items, single-event alias 400, and queue-failure internal rejections. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a canonical batch envelope format ( ChangesBatch envelope contract and implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/controllers/track.controller.ts (1)
613-621: ⚡ Quick winProject rejected items to the public response shape
Lines 613-621 push full internal rejected results (including
status) intorejected[]. Please map to{ index, reason, error }explicitly so the API contract doesn’t depend on serializer stripping behavior.Suggested patch
- const rejected: Extract<BatchItemResult, { status: 'rejected' }>[] = []; + const rejected: Array<{ + index: number; + reason: 'validation' | 'internal'; + error: string; + }> = []; for (const result of results) { if (result.status === 'accepted') { accepted += 1; } else { - rejected.push(result); + rejected.push({ + index: result.index, + reason: result.reason, + error: result.error, + }); } }🤖 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 `@apps/api/src/controllers/track.controller.ts` around lines 613 - 621, The loop collects full internal rejected BatchItemResult objects into rejected[], leaking internal fields like status; instead, map each rejected result to the public shape { index, reason, error } (e.g., use result.index, result.reason, result.error) and push that object into rejected[]; also update the rejected variable's type to reflect the public response shape rather than Extract<BatchItemResult,...> so the API contract is explicit.
🤖 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.
Nitpick comments:
In `@apps/api/src/controllers/track.controller.ts`:
- Around line 613-621: The loop collects full internal rejected BatchItemResult
objects into rejected[], leaking internal fields like status; instead, map each
rejected result to the public shape { index, reason, error } (e.g., use
result.index, result.reason, result.error) and push that object into rejected[];
also update the rejected variable's type to reflect the public response shape
rather than Extract<BatchItemResult,...> so the API contract is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1382b325-ad95-4782-a74c-2a1060c7a13e
📒 Files selected for processing (6)
FORK-PATCHES.mdapps/api/src/controllers/track.controller.tsapps/api/src/hooks/duplicate.hook.tsapps/api/src/routes/track-batch.router.test.tsapps/api/src/routes/track.router.tspackages/validation/src/track.validation.ts
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
Upstream review on Openpanel-dev/openpanel#377 asked for batch ingestion to ride the existing
/trackendpoint as{ "type": "batch", "payload": [event, ...] }instead of a separate/track/batchroute. This PR adopts that exact contract in our fork now, and also syncsupstream/main, so:What
POST /trackaccepts the batch envelope —{ "type": "batch", "payload": [...] }(up to 2000 events, 10 MB) dispatched through the sameprocessBatchpipeline as the legacy route: shared salts/geo per request, per-item validation, bounded concurrency (50),202 { accepted, rejected[] }with per-index errorsPOST /track/batchkept as a deprecated alias ({ "events": [...] }, markeddeprecatedin OpenAPI) for already-shipped clients — remove once traffic hits zeroduplicateHook, next to the existing replay skip) — offline-first SDKs retry whole batchesupstream/main(6d9445c) — only textual conflict waspackages/queue/src/queues.ts, resolved as upstream's new version minus theisTimestampFromThePastfield we removedFORK-PATCHES.mddocuments permanent fork differences (CI workflow) and every temporary divergence (worker session handling,eventMsbucketing, 5-day floor, the alias) plus the sync playbookeventMssession bucketing, worker session handlingCompatibility review of the upstream sync
events.incoming-event.ts,track.controller.ts,ids.ts,event.controller.ts, session-handler)@platformatic/kafka→kafkajs): the call surface used by the track controller (shouldUseKafka,produceIncomingEvent) is signature-identical; we never patchedkafka.tsevents.kafka-consumer.tscalls ourincomingEvent(payload)— our handler derives live-vs-historical internally, so kafka-path events get the same session semantics without the removedisTimestampFromThePastflagadd_member_org_user_unique) — runmigrate:deployon rollout; new CH MVs come fromcode-migrations/3-init-chTests (run on the merged tree)
apps/api: 73/73 passing — batch suite parametrized to run against both transports so the alias can't drift, plus envelope-only regressions (single-event 200, single alias 400, nested-batch rejection, internal-error rejections, all dispatch arms)apps/worker: 7/7 passingFuture upstream merge
When upstream lands Openpanel-dev#377 + their session rework: take upstream's side wholesale in the files listed under "Temporary patches" in
FORK-PATCHES.md, re-run the batch + worker suites, then delete the alias. Remaining fork delta after that: the deprecated/track/batchendpoint (+ its tests),FORK-PATCHES.md, and our CI workflow.Notes for reviewers
bodyLimiton/trackalso applies to single-event bodies (previously 1 MB default) — both shapes share the route; commented in code🤖 Generated with Claude Code