Release: revenue attribution, users LTV + email lookup, feedback pipeline, slack agent reliability and chart images#541
Release: revenue attribution, users LTV + email lookup, feedback pipeline, slack agent reliability and chart images#541izadoesdev wants to merge 135 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR centralizes AI query trait-filter resolution into a new
Confidence Score: 4/5The core refactor is sound — trait resolution is cleanly centralized, index bookkeeping in executeBatch is correct, and the session guard fix prevents accidental sign-outs. The two flagged items are edge-case inefficiencies that do not break the happy path. The unknown-type guard in resolveRequestTraitFilters lets an unnecessary identity-service round-trip happen before an unknown query type fails downstream, and the raw ClickHouse error strings now reach the LLM context without any sanitization pass. Neither causes incorrect data or broken flows on valid inputs, but both deserve a follow-up before the code sees heavy use with novel query types or noisy ClickHouse errors. packages/ai/src/query/trait-filters.ts (unknown-type guard) and packages/ai/src/ai/mcp/agent-tools.ts (raw error forwarding) are worth a second look. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant AI as AI Agent / Tool
participant MCP as MCP buildBatchQueryRequests
participant EX as executeBatch
participant TF as resolveRequestTraitFilters
participant ID as Identity Service (resolveTraitSegment)
participant CH as ClickHouse
AI->>MCP: "queries[] with trait:<key> filters"
MCP->>MCP: invalidFilterFieldError() per query
MCP-->>AI: invalid[] (bad field) + requests[] (ok)
AI->>EX: executeBatch(requests)
loop per request (parallel)
EX->>TF: resolveRequestTraitFilters(req)
alt has trait filters
TF->>ID: resolveTraitSegment(projectId, traitFilters)
ID-->>TF: profile_id[]
TF-->>EX: req with profile_id filter
else no trait filters
TF-->>EX: original req (no-op)
end
end
EX->>CH: union query (successful requests)
CH-->>EX: rows
EX-->>AI: BatchResult[] (with errors for failures)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant AI as AI Agent / Tool
participant MCP as MCP buildBatchQueryRequests
participant EX as executeBatch
participant TF as resolveRequestTraitFilters
participant ID as Identity Service (resolveTraitSegment)
participant CH as ClickHouse
AI->>MCP: "queries[] with trait:<key> filters"
MCP->>MCP: invalidFilterFieldError() per query
MCP-->>AI: invalid[] (bad field) + requests[] (ok)
AI->>EX: executeBatch(requests)
loop per request (parallel)
EX->>TF: resolveRequestTraitFilters(req)
alt has trait filters
TF->>ID: resolveTraitSegment(projectId, traitFilters)
ID-->>TF: profile_id[]
TF-->>EX: req with profile_id filter
else no trait filters
TF-->>EX: original req (no-op)
end
end
EX->>CH: union query (successful requests)
CH-->>EX: rows
EX-->>AI: BatchResult[] (with errors for failures)
Reviews (1): Last reviewed commit: "chore(staging): merge main" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const config = QueryBuilders[request.type]; | ||
| if (config && !isFilterFieldAllowed(config, "profile_id")) { | ||
| throw new TraitFilterError( | ||
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
config is undefined (unknown query type), the guard if (config && !isFilterFieldAllowed(config, "profile_id")) is a no-op — config is falsy so the throw is skipped. The function then calls resolveTraitSegment (a network round-trip to the identity service) before the query ultimately fails downstream with an "unknown type" error. The guard should also reject when the config is missing entirely.
| const config = QueryBuilders[request.type]; | |
| if (config && !isFilterFieldAllowed(config, "profile_id")) { | |
| throw new TraitFilterError( | |
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | |
| ); | |
| } | |
| const config = QueryBuilders[request.type]; | |
| if (!config || !isFilterFieldAllowed(config, "profile_id")) { | |
| throw new TraitFilterError( | |
| `Trait filters are not supported for ${request.type}. Query types that support them accept a profile_id filter.` | |
| ); | |
| } |
| data: r.data, | ||
| rowCount: r.data.length, | ||
| ...(r.error && { error: "Query failed" }), | ||
| ...(r.error && { error: r.error }), |
There was a problem hiding this comment.
"Query failed" to r.error means ClickHouse-level error messages (which can include table names, column names, and query fragments) are forwarded verbatim to the LLM context. For filter-validation errors this is intentional and useful, but a transient ClickHouse failure or a malformed query could expose internal schema details. Consider forwarding r.error as-is for known structured errors (e.g. TraitFilterError messages or the filter-field error format) and falling back to a sanitized string for raw database errors.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 521 files
Confidence score: 2/5
- In
apps/basket/src/hooks/auth.ts,_resolveOwnerIdnow appears to fail hard on member-query errors instead of returningnull, which can make owner lookup failures take down the whole auth lookup path and cause avoidable sign-in/access regressions—restore the previous fallback behavior (capture +null) before merging. - In
apps/api/src/http/errors.ts, production responses now unconditionally include raw Elysia validation messages, which can bypass the existingexposeStructuredsanitization intent and leak internal validation detail to clients—reapply the production gating so only sanitized fields are exposed. - In
apps/dashboard/components/layout/organization-selector.tsx, switching orgs callsqueryClient.clear(), which can wipe unrelated cache and pending mutations (billing/session/flags) and introduce cross-page state loss or flaky UX after an org switch—replace this with targeted invalidation/removal for org-scoped queries. - In
apps/docs/public/pricing.md, the dropped(Scale)label and removedScale vs Enterprisecross-reference can mislead customers if entitlement configs still usescale, increasing support and sales confusion—restore the tier naming/cross-reference to match current product configuration before release.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/http/errors.ts">
<violation number="1" location="apps/api/src/http/errors.ts:83">
P2: Raw Elysia validation messages are exposed unconditionally in production responses, bypassing the existing `exposeStructured` sanitization gate that protects `why`, `fix`, and `link`.
In production, `exposeStructured` is `false` for `ValidationError` (it is not an `EvlogError`), so the top-level `error` field safely falls back to "Invalid request". However, the `details` array containing up to 20 raw `issue.summary` / `issue.message` strings is still included. Those messages can leak schema field names, custom validation logic, header/cookie expectations, or reflected input text that the `getSafeErrorMessage` policy is meant to suppress.
Consider gating `details` behind the same `exposeStructured` check, or introducing an explicit allowlist/gate for which validation messages are safe to expose publicly.</violation>
</file>
<file name="apps/basket/src/hooks/auth.ts">
<violation number="1" location="apps/basket/src/hooks/auth.ts:56">
P1: Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. `_resolveOwnerId` previously returned `null` on member-query failures (after `captureError`); now it throws `websiteLookupUnavailable`, and that error propagates through `getWebsiteByIdWithOwnerCached` and `getWebsiteByIdV2`. Since `ownerId` is typed as nullable (`string | null`) and all callers treat it as optional enrichment—not a hard auth gate—`ownerId` resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down `/track` and `/identify` requests for otherwise-valid websites. Consider returning `null` from `_resolveOwnerId` on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside `getWebsiteByIdWithOwnerCached` and returning the website with `ownerId: null` so the optional enrichment doesn't block the auth path.</violation>
</file>
<file name="apps/dashboard/components/layout/organization-selector.tsx">
<violation number="1" location="apps/dashboard/components/layout/organization-selector.tsx:164">
P2: Organization switch now uses `queryClient.clear()`, which wipes the entire React Query cache including billing state, session metadata, feature flags, and any pending mutations. The previous code was more targeted: it removed only org-scoped query keys and explicitly invalidated the active-organization query so `useOrganizationsContext` promptly picked up the new org. Global `clear()` makes that refresh implicit and risks transient empty-state flicker during the redirect to `/websites`. Prefer restoring scoped invalidation — remove org-scoped keys and explicitly invalidate the active-organization query instead.</violation>
</file>
<file name="apps/docs/public/pricing.md">
<violation number="1" location="apps/docs/public/pricing.md:35">
P2: The product-limits table header lost the `(Scale)` annotation and the definitions section dropped the `Scale vs Enterprise` cross-reference. If internal tier IDs and entitlement configs still use `scale` — which the codebase suggests via `pricing/_pricing/estimator-scale.ts` — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring `Enterprise (Scale)` in the header or a short footnote) so reviewers know the internal plan id is still `scale`.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| message: "Failed to fetch workspace owner", | ||
| organizationId, | ||
| }); | ||
| throw basketErrors.websiteLookupUnavailable(); |
There was a problem hiding this comment.
P1: Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. _resolveOwnerId previously returned null on member-query failures (after captureError); now it throws websiteLookupUnavailable, and that error propagates through getWebsiteByIdWithOwnerCached and getWebsiteByIdV2. Since ownerId is typed as nullable (string | null) and all callers treat it as optional enrichment—not a hard auth gate—ownerId resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down /track and /identify requests for otherwise-valid websites. Consider returning null from _resolveOwnerId on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside getWebsiteByIdWithOwnerCached and returning the website with ownerId: null so the optional enrichment doesn't block the auth path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/basket/src/hooks/auth.ts, line 56:
<comment>Owner lookup failures are now fatal to the entire website auth lookup, which is a regression for ingest resilience. `_resolveOwnerId` previously returned `null` on member-query failures (after `captureError`); now it throws `websiteLookupUnavailable`, and that error propagates through `getWebsiteByIdWithOwnerCached` and `getWebsiteByIdV2`. Since `ownerId` is typed as nullable (`string | null`) and all callers treat it as optional enrichment—not a hard auth gate—`ownerId` resolution should remain a soft failure. Keeping it soft means a transient member-table issue won't take down `/track` and `/identify` requests for otherwise-valid websites. Consider returning `null` from `_resolveOwnerId` on non-EvlogError DB failures so the website lookup still succeeds, or at least catching the owner-resolution error inside `getWebsiteByIdWithOwnerCached` and returning the website with `ownerId: null` so the optional enrichment doesn't block the auth path.</comment>
<file context>
@@ -45,10 +46,14 @@ function _resolveOwnerId(
message: "Failed to fetch workspace owner",
organizationId,
});
+ throw basketErrors.websiteLookupUnavailable();
}
</file context>
| Checkout **Enterprise** maps to **Scale** entitlements in-app (same column below). | ||
|
|
||
| | | Free | Hobby | Pro | Enterprise (Scale) | | ||
| | | Free | Hobby | Pro | Enterprise | |
There was a problem hiding this comment.
P2: The product-limits table header lost the (Scale) annotation and the definitions section dropped the Scale vs Enterprise cross-reference. If internal tier IDs and entitlement configs still use scale — which the codebase suggests via pricing/_pricing/estimator-scale.ts — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring Enterprise (Scale) in the header or a short footnote) so reviewers know the internal plan id is still scale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/public/pricing.md, line 35:
<comment>The product-limits table header lost the `(Scale)` annotation and the definitions section dropped the `Scale vs Enterprise` cross-reference. If internal tier IDs and entitlement configs still use `scale` — which the codebase suggests via `pricing/_pricing/estimator-scale.ts` — this removal removes an important breadcrumb for keeping docs and code aligned. Future edits to either side can drift silently. Consider keeping a visible reminder (e.g., restoring `Enterprise (Scale)` in the header or a short footnote) so reviewers know the internal plan id is still `scale`.</comment>
<file context>
@@ -32,45 +32,33 @@ Overage = events **above** the monthly included amount. Cumulative overage is ch
-Checkout **Enterprise** maps to **Scale** entitlements in-app (same column below).
-
-| | Free | Hobby | Pro | Enterprise (Scale) |
+| | Free | Hobby | Pro | Enterprise |
| --- | --- | --- | --- | --- |
| Funnels | 1 | 5 | 50 | Unlimited |
</file context>
| | | Free | Hobby | Pro | Enterprise | | |
| +| | Free | Hobby | Pro | Enterprise (Scale) | |
There was a problem hiding this comment.
0 issues found across 7 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 51 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/rpc/src/routers/insights.ts">
<violation number="1" location="packages/rpc/src/routers/insights.ts:337">
P2: The `.catch(() => false)` wrapper around `withWorkspace` in `getById` and `related` swallows every rejection, including database and infrastructure errors. After reviewing `resolveWorkspace` in `packages/rpc/src/procedures/with-workspace.ts`, it calls `getMemberRole` and `context.getBilling()` which can throw on DB/connectivity failures. When that happens the endpoint will return `{ success: true, insight: null }` (or an empty array) instead of a real error, masking outages and bypassing telemetry.
Preserve the anti-enumeration behavior for actual permission denials, but rethrow unexpected/infrastructure errors so they surface properly. A simple approach is to inspect the rejection and only swallow the expected `rpcError.forbidden` / `rpcError.unauthorized` kinds.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 46 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/dashboard/lib/safe-callback.ts">
<violation number="1" location="apps/dashboard/lib/safe-callback.ts:1">
P1: `safeCallbackPath` can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like `/\t/attacker.example` passes the `startsWith("/")` and `!startsWith("//")` checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes `//attacker.example`, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with `new URL()` and verifying the resulting pathname has not collapsed into a protocol-relative form.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,14 @@ | |||
| export function safeCallbackPath( | |||
There was a problem hiding this comment.
P1: safeCallbackPath can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like /\t/attacker.example passes the startsWith("/") and !startsWith("//") checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes //attacker.example, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with new URL() and verifying the resulting pathname has not collapsed into a protocol-relative form.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/lib/safe-callback.ts, line 1:
<comment>`safeCallbackPath` can be bypassed via URL-parser-stripped whitespace, leading to an open redirect. A value like `/\t/attacker.example` passes the `startsWith("/")` and `!startsWith("//")` checks because the tab character sits between the slashes. When the browser or Next.js router parses the URL later, the tab is stripped (per the WHATWG URL Standard) and the path becomes `//attacker.example`, which is a protocol-relative external URL. Consider rejecting tab, LF, and CR characters explicitly before the prefix test, or parsing the string against a known base origin with `new URL()` and verifying the resulting pathname has not collapsed into a protocol-relative form.</comment>
<file context>
@@ -0,0 +1,14 @@
+export function safeCallbackPath(
+ callback: string | null | undefined,
+ fallback = "/websites"
</file context>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 22 unresolved issues from previous reviews.
Re-trigger cubic
1 participant
What's in this release
Users page
profile_listRevenue attribution
invoice.paid/invoice.payment_succeededhandling: recurring subscription payments now attribute to sessions and identified users. Metadata is read from the invoice,subscription_details, andparent.subscription_details(covers Autumn and newer Stripe API versions where payment intents carry no metadata)invoice.paidas a required webhook eventFeedback pipeline
website_idfkey indexed@databuddy/serviceswith source-aware Slack alertssubmit_feedbackagent tool for dashboard chat and the Slack bot, with consent-aware prompting and a single source of truth for the prompt rulesfeedback-previewchat component with send and receipt modesSlack agent
agent_runevents never reaching Axiom when a deploy landed mid-answerslack_run_timed_outtelemetry instead of sitting on "Thinking..." until the next deploy kills them@databuddy/chartspackage renders the agent's chart components (line, area, bar, stacked bar, pie, donut) to dashboard-themed PNGs server-side (ECharts SSR + resvg, LT Superior fonts, dark/light design tokens). Slack answers upload up to 3 charts into the thread and fall back to the existing text tables if rendering or upload fails. Activating uploads requires adding thefiles:writescope to the Slack appAgent & AI
list_profile_traitstool: trait key/value distribution with profile counts, guiding agents totrait:<key>segmentation before queryingget_data, MCP tools, and raw SQL paths; telemetry gated on the exported sanitized constant; error messaging keyed on error type, not input shapeDashboard & misc
has_active_subscriptiontrait dropped (redundant withplan)Custom event page path (new)
pathand basket persists it tocustom_events.path. The column existed but was never populated (0 of 144,409 custom events over the prior 30 days had a path).Hash-route pageviews (new)
screen_viewevents now include the URL hash fragment inpathwhentrackHashChangesis enabled. PreviouslybuildPagePathused pathname only and dropped the hash. This also fixes a pre-existing failing tracker E2E test.Insights becomes Findings (new)
@databuddy/ai,rpc,redis,db,shared, andevals.Misc (new)