feat: agent org workspace + billing & auth robustness#464
Open
izadoesdev wants to merge 26 commits into
Open
Conversation
…ceiling Set accountLinking.requireLocalEmailVerified: true explicitly so future config changes can't silently regress the OAuth pre-account-takeover protection. Bump maxPasswordLength from 32 to 128 to align with NIST 800-63B and unblock password manager output.
Outstanding reset-password verification rows previously survived both a successful reset and a settings-driven password change, leaving the old link usable until its 1-hour TTL. Add an onPasswordReset hook and a databaseHooks.account.update.after hook on the credential account so both flows purge the user's outstanding reset tokens. Also enable revokeSessionsOnPasswordReset so existing sessions die when the email reset flow completes.
With cookieCache enabled, Better-Auth validates requests against the signed sessionData cookie payload for up to maxAge seconds without hitting Redis. That left a 5-minute window where a stolen cookie kept working after logout, password reset, ban-user, or organization member removal. secondaryStorage already points at Redis, so dropping the cache costs one fast round-trip per request and closes the post-revocation replay window entirely.
Emit an explicit warn at the point of detection so blocked-ingest rows carry customerId, featureId, properties, and usage/grant fields. The wide-event context was previously lost by the time the error reached Axiom, leaving 114k quota-exceeded rows with no attribution.
Remove restating JSDoc across the basket lib/utils modules (one comment on validateRequest was also stale, describing a return shape the function no longer has). Promote AnalyticsEventInput and OutgoingLinkInput to exported z.infer types in the validation package and reuse them in the route handlers instead of re-deriving locally.
…ding Sanitize the parsed body in a single pass and reuse one autumn handler instance. Extract loadSession so a getSession failure is logged and rethrown instead of being swallowed into a null identity, and drop the manual activeOrganizationId cast now that the session type carries it.
Collapse the near-identical auto-topup, usage-alert, and spend-limit mutation handlers into one generic upsertBillingControl that handles the owner check, getOrCreate/merge/update, and error logging. Replaces the three parallel entry interfaces with a single keyed BillingControlEntries.
Detect week-over-week conversion drops on funnel definitions and goals via a new funnel-detection module, reusing the shared wowWindow helper and makeWowSignal (both lifted out of detection.ts) and enrichment's window logic. Extract insight persistence and dedup out of generation.ts into persistence.ts, and expose the rpc analytics-utils entrypoint that funnel detection needs for funnel/goal analytics processing.
Transient DB failures in getWebsiteById and getOrganizationRole were caught and degraded to null, surfacing as misleading 404/403 responses that hid the real incident. Let errors propagate to the ORPC handler.
Better-Auth wraps internal failures (e.g. session-refresh DB writes) as a generic "Failed to get session" and logs the real cause only to its default console logger, so the underlying error never reached Axiom. Wire a logger.log forwarder to evlog to surface the actual error.
A transient DB failure in _getOrganizationOwnerId was caught and degraded to null, which fell back to billing the member instead of the org owner. Let the error propagate so identity is never silently wrong.
Names and signatures already document these pure helpers; the JSDoc only restated them and the example blocks duplicated the same call pattern.
Replace unchecked `as` casts of untrusted webhook data with Zod schemas, deriving the payload types via z.infer. Malformed payloads now return a logged 400 instead of crashing deep inside a handler.
Add a resolveInsightsBilling DI seam that resolves the billing customer and checks agent credits before running the website agent, skipping generation when credits are exhausted. Track usage against Autumn after each agent run. Align getComparisonPeriod with the detection wowWindow so the agent and signal detection share the same week-over-week window.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
|
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
|
Contributor
Greptile SummaryThis PR adds credit-gating and usage billing to the insights agent pipeline, introduces funnel/goal WoW signal detection, aligns
Confidence Score: 3/5The core billing gate and funnel detection are well-structured and tested, but the error-propagation change in the Autumn identify callback could surface as unexpected 500s on billing requests during transient auth outages. The insights billing DI seam, funnel detection, and auth hardening are solid. The riskiest part is apps/api/src/billing/autumn.ts: the identify callback used to swallow all session errors (returning null); it now re-throws them, and whether autumnHandler from the Autumn SDK catches exceptions from its identify callback is not clear from this diff. If it doesn't, any transient getSession failure becomes a 500 on every Autumn proxy request. The CUSTOM goal type cast in funnel-detection.ts and the return input shortcut in billing control handlers are lower-risk but worth addressing. apps/api/src/billing/autumn.ts (identify error propagation), apps/insights/src/funnel-detection.ts (CUSTOM goal type cast) Important Files Changed
Sequence DiagramsequenceDiagram
participant Cron
participant generateWebsiteInsights
participant resolveInsightsBilling
participant Autumn
participant analyzeWebsite
participant trackAgentUsageAndBill
Cron->>generateWebsiteInsights: run(websiteId, orgId)
generateWebsiteInsights->>resolveInsightsBilling: principal(orgId, userId)
resolveInsightsBilling->>Autumn: resolveBillingCustomerId
Autumn-->>resolveInsightsBilling: "customerId | null"
resolveInsightsBilling->>Autumn: ensureAgentCreditsAvailable(customerId)
Autumn-->>resolveInsightsBilling: allowed: boolean
resolveInsightsBilling-->>generateWebsiteInsights: "{allowed, billingCustomerId}"
alt not allowed
generateWebsiteInsights-->>Cron: skipped_no_credits
else allowed
generateWebsiteInsights->>analyzeWebsite: "{billingCustomerId, ...}"
analyzeWebsite->>analyzeWebsite: detectSignals + detectFunnelGoalSignals
analyzeWebsite->>analyzeWebsite: agent.generate()
analyzeWebsite->>trackAgentUsageAndBill: "{usage, modelId, billingCustomerId}"
trackAgentUsageAndBill->>Autumn: bill usage
analyzeWebsite-->>generateWebsiteInsights: insights
generateWebsiteInsights-->>Cron: succeeded
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+87
to
111
| async function identifyAutumnCustomer(request: Request) { | ||
| const session = await loadSession(request); | ||
| if (!session?.user) { | ||
| return null; | ||
| } | ||
|
|
||
| const activeOrgId = ( | ||
| session.session as { activeOrganizationId?: string | null } | ||
| )?.activeOrganizationId; | ||
| const activeOrgId = session.session.activeOrganizationId ?? null; | ||
|
|
||
| if (activeOrgId) { | ||
| const role = await getMemberRole(session.user.id, activeOrgId); | ||
| if (role !== "owner" && role !== "admin") { | ||
| return null; | ||
| } | ||
| if (activeOrgId) { | ||
| const role = await getMemberRole(session.user.id, activeOrgId); | ||
| if (role !== "owner" && role !== "admin") { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| const customerId = await getBillingCustomerId(session.user.id, activeOrgId); | ||
| const customerId = await getBillingCustomerId(session.user.id, activeOrgId); | ||
|
|
||
| return { | ||
| customerId, | ||
| customerData: { | ||
| name: session.user.name, | ||
| email: session.user.email, | ||
| }, | ||
| }; | ||
| } catch (error) { | ||
| useLogger().error( | ||
| error instanceof Error ? error : new Error(String(error)), | ||
| { | ||
| autumn: "identify", | ||
| } | ||
| ); | ||
| return null; | ||
| } | ||
| return { | ||
| customerId, | ||
| customerData: { | ||
| name: session.user.name, | ||
| email: session.user.email, | ||
| }, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
identify callback
loadSession now throws on any getSession failure, and identifyAutumnCustomer no longer wraps the call. The old code always returned null | CustomerData, meaning the autumnHandler's identify callback could never throw. The autumn SDK isn't guaranteed to catch exceptions thrown by the identify function — if it doesn't, a transient session-store error will produce a 500 response instead of gracefully treating the request as unauthenticated. This affects every Autumn billing request during auth-service instability.
Comment on lines
+134
to
+138
| goalConversion: async (goal, range) => { | ||
| const steps: AnalyticsStep[] = [ | ||
| { | ||
| step_number: 1, | ||
| type: goal.type as "PAGE_VIEW" | "EVENT", |
Contributor
There was a problem hiding this comment.
CUSTOM goal type cast bypasses type safety
GoalDef.type is declared as "PAGE_VIEW" | "EVENT" | "CUSTOM" and fetchGoals applies no type filter, so a CUSTOM goal is valid input here. The as "PAGE_VIEW" | "EVENT" cast suppresses the TypeScript error but doesn't change the runtime value — "CUSTOM" is still passed to processGoalAnalytics, which only expects the two analytics step types. This produces either incorrect conversion numbers or a thrown exception (silently swallowed by the outer catch) for any CUSTOM goal. A type !== "CUSTOM" guard on the fetch or before processing would prevent the unsound cast.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 26 files
Confidence score: 3/5
- There is meaningful regression risk from error-handling changes in
packages/rpc/src/utils/billing.tsandapps/api/src/billing/autumn.ts: both now rethrow lookup failures instead of degrading gracefully, which can turn previously resilient request paths into hard failures. apps/insights/src/persistence.tshas a concrete behavior bug: refreshed insights do not updatecreatedAt, so cooldown-based dedupe can expire for rows that are still actively refreshed.- Additional medium-risk correctness concerns in
apps/insights/src/funnel-detection.ts(unsafe narrowing cast andLIMITwithoutORDER BY) may cause unstable or invalid analytics outputs, while lower-severity notes inpackages/auth/src/auth.tsandapps/basket/src/lib/billing.tsare more performance/privacy hygiene than merge blockers. - Pay close attention to
packages/rpc/src/utils/billing.ts,apps/api/src/billing/autumn.ts,apps/insights/src/persistence.ts, andapps/insights/src/funnel-detection.ts- these paths contain the main user-impacting reliability and analytics correctness risks.
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/auth/src/auth.ts">
<violation number="1" location="packages/auth/src/auth.ts:166">
P2: The new reset-token purge query filters on unindexed verification columns, which can cause full-table scans under load.</violation>
</file>
<file name="apps/basket/src/lib/billing.ts">
<violation number="1" location="apps/basket/src/lib/billing.ts:42">
P2: Logging the raw `properties` object could expose sensitive data from future callers. Consider omitting `properties` from the warn log or destructuring only known safe fields (e.g., `website_id`, `api_route`). The billing context (`usage`, `granted`, `unlimited`) already provides enough diagnostic signal for the exceeded-quota case.</violation>
</file>
<file name="packages/rpc/src/utils/billing.ts">
<violation number="1" location="packages/rpc/src/utils/billing.ts:19">
P1: This owner lookup now throws on DB errors instead of degrading to `null`, which can fail request paths that previously fell back safely.</violation>
</file>
<file name="apps/insights/src/persistence.ts">
<violation number="1" location="apps/insights/src/persistence.ts:243">
P1: Refreshing an existing insight does not bump `createdAt`, so cooldown-based dedupe can expire for actively refreshed rows.</violation>
</file>
<file name="apps/insights/src/funnel-detection.ts">
<violation number="1" location="apps/insights/src/funnel-detection.ts:75">
P2: Avoid unsafe narrowing cast on funnel step type; validate/map runtime values before building `AnalyticsStep`.
(Based on your team's feedback about avoiding unsafe type casts.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/insights/src/funnel-detection.ts:100">
P2: Applying `LIMIT` without `ORDER BY` makes the analyzed funnel subset non-deterministic, which can cause signal results to fluctuate across runs for sites with many active funnels.</violation>
</file>
<file name="apps/api/src/billing/autumn.ts">
<violation number="1" location="apps/api/src/billing/autumn.ts:83">
P1: Rethrowing session lookup errors makes Autumn requests fail hard instead of degrading to anonymous customer resolution.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (App/Web)
participant API as API Route (Elysia)
participant Auth as Auth Service (better-auth)
participant Insights as Insights Generation
participant Billing as resolveInsightsBilling
participant Autumn as Autumn Billing API
participant DB as Database (Postgres)
participant Redis as Redis Cache
Note over Client,Redis: NEW: Credit-gated insight generation
Client->>API: POST /generate-website-insights
API->>Auth: getSession(headers)
Auth-->>API: session with activeOrganizationId
API->>Billing: resolveInsightsBilling({ organizationId, userId })
alt No billing customer found
Billing->>Billing: resolveBillingCustomerId → null
Billing->>Billing: ensureCreditsAvailable(null) → allowed
Billing-->>API: { allowed: true, billingCustomerId: null }
else Customer found, credits available
Billing->>Autumn: resolveAgentBillingCustomerId(principal)
Autumn-->>Billing: customerId
Billing->>Autumn: ensureAgentCreditsAvailable(customerId)
Autumn-->>Billing: true
Billing-->>API: { allowed: true, billingCustomerId }
else Customer found, credits exhausted
Billing->>Autumn: ensureAgentCreditsAvailable(customerId)
Autumn-->>Billing: false
Billing-->>API: { allowed: false, billingCustomerId }
end
alt Not allowed (skipped_no_credits)
API->>API: Emit evlog event "skipped_no_credits"
API-->>Client: { status: "skipped", reason: "no_credits" }
else Allowed
API->>Insights: generateWebsiteInsights()
Insights->>DB: fetchFunnels() / fetchGoals()
DB-->>Insights: funnel/goal definitions
Insights->>Insights: detectSignals() + detectFunnelGoalSignals()
Insights->>DB: fetch contexts, org history
DB-->>Insights: context data
Insights->>Insights: agent.run() with model
Insights->>Autumn: trackAgentUsageAndBill({ usage, modelId, billingCustomerId })
Insights-->>API: generated insights array
API->>DB: persistWebsiteInsights() (dedupe + upsert)
API->>Redis: invalidateInsightsCachesForOrganization()
API-->>Client: { status: "succeeded", insights }
end
Note over API,Redis: CHANGED: Wow window shared with detection
API->>Insights: getComparisonPeriod(lookbackDays)
Insights->>Insights: wowWindow(today, lookbackDays) → max(3, lookback), no 90-day cap
Insights-->>API: WeekOverWeekPeriod
API->>DB: Queries use consistent non-overlapping windows
DB-->>API: results
Note over Auth,DB: HARDENED: Auth session/reset flows
Auth->>DB: onPasswordReset(user.id)
Auth->>DB: purgeOutstandingResetTokens(user.id)
Auth->>DB: revokeSessionsOnPasswordReset
Auth->>Auth: disable cookie cache (session.cookieCache.enabled=false)
Auth->>Auth: requireLocalEmailVerified=true for account linking
Auth->>DB: account.update hook → purge tokens
DB-->>Auth: updated
Note over API,Autumn: HARDENED: Webhook validation
Autumn->>API: POST /webhooks/autumn
API->>API: dispatch(event.type)
alt balance.limit_reached
API->>API: limitReachedSchema.parse(data)
else usage_alert_triggered
API->>API: usageAlertSchema.parse(data)
else customer.products.updated
API->>API: productsUpdatedSchema.parse(data)
end
alt Zod validation error
API-->>Autumn: 400 { success: false }
else Valid payload
API->>Auth: identifyAutumnCustomer()
Auth-->>API: customerId
API->>Autumn: processWebhook(customerId)
Autumn-->>API: result
API-->>Autumn: 200
end
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Comment on lines
+19
to
+23
| const orgMember = await db.query.member.findFirst({ | ||
| where: { organizationId, role: "owner" }, | ||
| columns: { userId: true }, | ||
| }); | ||
| return orgMember?.userId ?? null; |
There was a problem hiding this comment.
P1: This owner lookup now throws on DB errors instead of degrading to null, which can fail request paths that previously fell back safely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/rpc/src/utils/billing.ts, line 19:
<comment>This owner lookup now throws on DB errors instead of degrading to `null`, which can fail request paths that previously fell back safely.</comment>
<file context>
@@ -16,16 +16,11 @@ const _getOrganizationOwnerId = async (
- logger.error({ error }, "Error resolving organization owner");
- return null;
- }
+ const orgMember = await db.query.member.findFirst({
+ where: { organizationId, role: "owner" },
+ columns: { userId: true },
</file context>
Suggested change
| const orgMember = await db.query.member.findFirst({ | |
| where: { organizationId, role: "owner" }, | |
| columns: { userId: true }, | |
| }); | |
| return orgMember?.userId ?? null; | |
| try { | |
| const orgMember = await db.query.member.findFirst({ | |
| where: { organizationId, role: "owner" }, | |
| columns: { userId: true }, | |
| }); | |
| return orgMember?.userId ?? null; | |
| } catch (error) { | |
| logger.error({ error }, "Error resolving organization owner"); | |
| return null; | |
| } |
| } | ||
| await Promise.all( | ||
| toRefresh.map(({ id, row }) => | ||
| db.update(analyticsInsights).set(row).where(eq(analyticsInsights.id, id)) |
There was a problem hiding this comment.
P1: Refreshing an existing insight does not bump createdAt, so cooldown-based dedupe can expire for actively refreshed rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/persistence.ts, line 243:
<comment>Refreshing an existing insight does not bump `createdAt`, so cooldown-based dedupe can expire for actively refreshed rows.</comment>
<file context>
@@ -0,0 +1,292 @@
+ }
+ await Promise.all(
+ toRefresh.map(({ id, row }) =>
+ db.update(analyticsInsights).set(row).where(eq(analyticsInsights.id, id))
+ )
+ );
</file context>
| autumn: "identify", | ||
| autumn_stage: "getSession", | ||
| }); | ||
| throw err; |
There was a problem hiding this comment.
P1: Rethrowing session lookup errors makes Autumn requests fail hard instead of degrading to anonymous customer resolution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/billing/autumn.ts, line 83:
<comment>Rethrowing session lookup errors makes Autumn requests fail hard instead of degrading to anonymous customer resolution.</comment>
<file context>
@@ -48,69 +48,64 @@ async function stripPrivilegedBody(request: Request): Promise<Request> {
+ autumn: "identify",
+ autumn_stage: "getSession",
+ });
+ throw err;
+ }
+}
</file context>
Suggested change
| throw err; | |
| return null; |
| .where( | ||
| and( | ||
| like(verificationTable.identifier, "reset-password:%"), | ||
| eq(verificationTable.value, userId) |
There was a problem hiding this comment.
P2: The new reset-token purge query filters on unindexed verification columns, which can cause full-table scans under load.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/auth/src/auth.ts, line 166:
<comment>The new reset-token purge query filters on unindexed verification columns, which can cause full-table scans under load.</comment>
<file context>
@@ -155,6 +156,26 @@ function notifySignUpSlackAction(input: {
+ .where(
+ and(
+ like(verificationTable.identifier, "reset-password:%"),
+ eq(verificationTable.value, userId)
+ )
+ );
</file context>
| }); | ||
|
|
||
| if (!response.allowed) { | ||
| log.warn("Event quota exceeded", { |
There was a problem hiding this comment.
P2: Logging the raw properties object could expose sensitive data from future callers. Consider omitting properties from the warn log or destructuring only known safe fields (e.g., website_id, api_route). The billing context (usage, granted, unlimited) already provides enough diagnostic signal for the exceeded-quota case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/basket/src/lib/billing.ts, line 42:
<comment>Logging the raw `properties` object could expose sensitive data from future callers. Consider omitting `properties` from the warn log or destructuring only known safe fields (e.g., `website_id`, `api_route`). The billing context (`usage`, `granted`, `unlimited`) already provides enough diagnostic signal for the exceeded-quota case.</comment>
<file context>
@@ -39,6 +39,16 @@ export function checkAutumnUsage(
});
if (!response.allowed) {
+ log.warn("Event quota exceeded", {
+ customerId,
+ featureId,
</file context>
| function toAnalyticsSteps(steps: FunnelStep[]): AnalyticsStep[] { | ||
| return steps.map((step, index) => ({ | ||
| step_number: index + 1, | ||
| type: step.type as "PAGE_VIEW" | "EVENT", |
There was a problem hiding this comment.
P2: Avoid unsafe narrowing cast on funnel step type; validate/map runtime values before building AnalyticsStep.
(Based on your team's feedback about avoiding unsafe type casts.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/funnel-detection.ts, line 75:
<comment>Avoid unsafe narrowing cast on funnel step type; validate/map runtime values before building `AnalyticsStep`.
(Based on your team's feedback about avoiding unsafe type casts.) </comment>
<file context>
@@ -0,0 +1,256 @@
+function toAnalyticsSteps(steps: FunnelStep[]): AnalyticsStep[] {
+ return steps.map((step, index) => ({
+ step_number: index + 1,
+ type: step.type as "PAGE_VIEW" | "EVENT",
+ target: step.target,
+ name: step.name,
</file context>
| sql`jsonb_array_length(${funnelDefinitions.steps}) > 1` | ||
| ) | ||
| ) | ||
| .limit(MAX_DEFINITIONS), |
There was a problem hiding this comment.
P2: Applying LIMIT without ORDER BY makes the analyzed funnel subset non-deterministic, which can cause signal results to fluctuate across runs for sites with many active funnels.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/insights/src/funnel-detection.ts, line 100:
<comment>Applying `LIMIT` without `ORDER BY` makes the analyzed funnel subset non-deterministic, which can cause signal results to fluctuate across runs for sites with many active funnels.</comment>
<file context>
@@ -0,0 +1,256 @@
+ sql`jsonb_array_length(${funnelDefinitions.steps}) > 1`
+ )
+ )
+ .limit(MAX_DEFINITIONS),
+ fetchGoals: () =>
+ db
</file context>
The insights generation pipeline bills agent usage via trackAgentUsageAndBill; widen the source union so it can identify itself as the insights source.
Satisfy the lint assist rules (sorted interface members and JSX attributes) that were failing CI on the pricing table components.
izadoesdev
changed the title
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 7 unresolved issues from previous reviews.
Re-trigger cubic
Make agent_chats.website_id nullable, index by organization instead of website, and switch the website FK to ON DELETE SET NULL so chats survive website deletion. Agent chats are now an organization-level workspace.
List and authorize agent chats against the organization workspace rather than a single website, and tolerate chats with a null website.
Add a list-websites tool and thread accessible-website context (accessible websites + default website id) through the toolkit so the agent can query across an organization's sites instead of a single hard-coded website.
Resolve the agent's accessible websites and default website from the organization context so a single chat can operate across the workspace's sites instead of one fixed website.
Move the agent out from under a single website into a top-level workspace: add a global /agent route with chat history, mentions for referencing specific websites, and a no-websites empty state. Chats are now scoped to the organization, so switching websites no longer loses the conversation.
There was a problem hiding this comment.
14 issues found across 46 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/auth/src/auth.ts">
<violation number="1" location="packages/auth/src/auth.ts:166">
P2: The new reset-token purge query filters on unindexed verification columns, which can cause full-table scans under load.</violation>
</file>
<file name="apps/basket/src/lib/billing.ts">
<violation number="1" location="apps/basket/src/lib/billing.ts:42">
P2: Logging the raw `properties` object could expose sensitive data from future callers. Consider omitting `properties` from the warn log or destructuring only known safe fields (e.g., `website_id`, `api_route`). The billing context (`usage`, `granted`, `unlimited`) already provides enough diagnostic signal for the exceeded-quota case.</violation>
</file>
<file name="packages/rpc/src/utils/billing.ts">
<violation number="1" location="packages/rpc/src/utils/billing.ts:19">
P1: This owner lookup now throws on DB errors instead of degrading to `null`, which can fail request paths that previously fell back safely.</violation>
</file>
<file name="apps/insights/src/funnel-detection.ts">
<violation number="1" location="apps/insights/src/funnel-detection.ts:75">
P2: Avoid unsafe narrowing cast on funnel step type; validate/map runtime values before building `AnalyticsStep`.
(Based on your team's feedback about avoiding unsafe type casts.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/insights/src/funnel-detection.ts:100">
P2: Applying `LIMIT` without `ORDER BY` makes the analyzed funnel subset non-deterministic, which can cause signal results to fluctuate across runs for sites with many active funnels.</violation>
</file>
<file name="apps/api/src/billing/autumn.ts">
<violation number="1" location="apps/api/src/billing/autumn.ts:83">
P1: Rethrowing session lookup errors makes Autumn requests fail hard instead of degrading to anonymous customer resolution.</violation>
</file>
<file name="apps/insights/src/persistence.ts">
<violation number="1" location="apps/insights/src/persistence.ts:243">
P1: Refreshing an existing insight does not bump `createdAt`, so cooldown-based dedupe can expire for actively refreshed rows.</violation>
</file>
<file name="apps/dashboard/components/agent/agent-mention-menu.tsx">
<violation number="1" location="apps/dashboard/components/agent/agent-mention-menu.tsx:6">
P2: This menu bypasses the dashboard DS menu boundary; implement it with `components/ds` `DropdownMenu` primitives instead of custom `div/ul/li` structure and `@databuddy/ui` controls.</violation>
</file>
<file name="packages/ai/src/ai/tools/execute-sql-query.ts">
<violation number="1" location="packages/ai/src/ai/tools/execute-sql-query.ts:113">
P1: `websiteId` switching is not fail-closed when `accessibleWebsites` is absent, so this new path can accept arbitrary site IDs without workspace membership checks.</violation>
</file>
<file name="apps/dashboard/app/(main)/agent/_components/global-agent-page.tsx">
<violation number="1" location="apps/dashboard/app/(main)/agent/_components/global-agent-page.tsx:14">
P3: This repeats the same `organizationId/hasWebsites/isLoading` gating and skeleton UI already present in `global-agent-redirect.tsx`; consider extracting a shared guard/render helper to avoid logic drift.</violation>
</file>
<file name="packages/ai/src/ai/tools/search-console.ts">
<violation number="1" location="packages/ai/src/ai/tools/search-console.ts:113">
P2: `search_console` now hard-requires `experimental_context` even when a domain is already provided. This can throw early and break callers that pass `domain` directly.</violation>
<violation number="2" location="packages/ai/src/ai/tools/search-console.ts:114">
P2: `search_console` ignores `websiteId` whenever `params.domain` is present, so multi-website queries can silently return metrics for the wrong site.</violation>
</file>
<file name="packages/ai/src/ai/tools/get-data.ts">
<violation number="1" location="packages/ai/src/ai/tools/get-data.ts:193">
P2: Result keys are order-dependent for multi-website queries, which can overwrite same-type entries and make website-to-result mapping inconsistent.</violation>
</file>
<file name="packages/ai/src/ai/config/context.ts">
<violation number="1" location="packages/ai/src/ai/config/context.ts:36">
P2: Attribute escaping is incomplete; only quotes are escaped, so `&`, `<`, and `>` in website metadata can break or inject into the generated context markup.</violation>
<violation number="2" location="packages/ai/src/ai/config/context.ts:51">
P2: `w.id` is interpolated into an XML-like attribute without escaping, unlike the other attributes.</violation>
</file>
<file name="packages/ai/src/ai/tools/utils/context.ts">
<violation number="1" location="packages/ai/src/ai/tools/utils/context.ts:31">
P1: Website authorization is bypassed when `accessibleWebsites` is empty/undefined, allowing arbitrary `websiteId` to be accepted.</violation>
</file>
<file name="apps/dashboard/app/(main)/websites/[id]/agent/_components/agent-page-client.tsx">
<violation number="1" location="apps/dashboard/app/(main)/websites/[id]/agent/_components/agent-page-client.tsx:13">
P2: `AgentPageClient` renders the chat workspace before organization context is resolved, so it can pass `null` as `organizationId` and show a transient/wrong "No active workspace" state. Gate rendering until `activeOrganizationId` is available.</violation>
</file>
<file name="packages/ai/src/ai/tools/scrape-page.ts">
<violation number="1" location="packages/ai/src/ai/tools/scrape-page.ts:226">
P1: The new `websiteId` flow is fail-open when `accessibleWebsites` is missing, allowing arbitrary website IDs to be resolved and scraped. Enforce explicit membership/default-website checks before resolving domain.</violation>
</file>
<file name="apps/dashboard/app/(main)/agent/_components/global-agent-redirect.tsx">
<violation number="1" location="apps/dashboard/app/(main)/agent/_components/global-agent-redirect.tsx:14">
P2: Redirecting on `chatId` alone also fires for orgs with no websites. Gate the redirect on `hasWebsites` to avoid jumping to `/agent/:chatId` in the no-websites state.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| execute: ({ sql, params }, options): Promise<QueryResult> => { | ||
| execute: ({ sql, websiteId, params }, options): Promise<QueryResult> => { | ||
| const ctx = getAppContext(options); | ||
| const resolved = resolveToolWebsite(ctx, websiteId); |
There was a problem hiding this comment.
P1: websiteId switching is not fail-closed when accessibleWebsites is absent, so this new path can accept arbitrary site IDs without workspace membership checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/tools/execute-sql-query.ts, line 113:
<comment>`websiteId` switching is not fail-closed when `accessibleWebsites` is absent, so this new path can accept arbitrary site IDs without workspace membership checks.</comment>
<file context>
@@ -90,18 +95,25 @@ Gotchas: timestamp column is "time" in events, "timestamp" elsewhere. Pageviews
- execute: ({ sql, params }, options): Promise<QueryResult> => {
+ execute: ({ sql, websiteId, params }, options): Promise<QueryResult> => {
const ctx = getAppContext(options);
+ const resolved = resolveToolWebsite(ctx, websiteId);
return executeAgentSqlForWebsite({
- websiteId: ctx.websiteId,
</file context>
Suggested change
| const resolved = resolveToolWebsite(ctx, websiteId); | |
| const resolved = resolveToolWebsite(ctx, websiteId); | |
| if ( | |
| websiteId && | |
| (ctx.accessibleWebsites ?? []).length === 0 && | |
| websiteId !== ctx.websiteId | |
| ) { | |
| throw new Error( | |
| "Website \"" + websiteId + "\" is not in this workspace. Call list_websites to see available websites." | |
| ); | |
| } |
|
|
||
| if (inputWebsiteId) { | ||
| if ( | ||
| accessible.length > 0 && |
There was a problem hiding this comment.
P1: Website authorization is bypassed when accessibleWebsites is empty/undefined, allowing arbitrary websiteId to be accepted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/tools/utils/context.ts, line 31:
<comment>Website authorization is bypassed when `accessibleWebsites` is empty/undefined, allowing arbitrary `websiteId` to be accepted.</comment>
<file context>
@@ -11,3 +11,44 @@ export function getAppContext(options: {
+
+ if (inputWebsiteId) {
+ if (
+ accessible.length > 0 &&
+ !accessible.some((w) => w.id === inputWebsiteId)
+ ) {
</file context>
| execute: ({ path }) => scrapePage(domain, path), | ||
| execute: async ({ websiteId, path }, options) => { | ||
| const ctx = getAppContext(options); | ||
| const resolved = resolveToolWebsite(ctx, websiteId); |
There was a problem hiding this comment.
P1: The new websiteId flow is fail-open when accessibleWebsites is missing, allowing arbitrary website IDs to be resolved and scraped. Enforce explicit membership/default-website checks before resolving domain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/tools/scrape-page.ts, line 226:
<comment>The new `websiteId` flow is fail-open when `accessibleWebsites` is missing, allowing arbitrary website IDs to be resolved and scraped. Enforce explicit membership/default-website checks before resolving domain.</comment>
<file context>
@@ -202,17 +204,33 @@ export async function getCachedSiteContext(
- execute: ({ path }) => scrapePage(domain, path),
+ execute: async ({ websiteId, path }, options) => {
+ const ctx = getAppContext(options);
+ const resolved = resolveToolWebsite(ctx, websiteId);
+ const domain =
+ resolved.domain || (await getWebsiteDomain(resolved.websiteId));
</file context>
Suggested change
| const resolved = resolveToolWebsite(ctx, websiteId); | |
| const allowedIds = new Set([ | |
| ...((ctx.accessibleWebsites ?? []).map((w) => w.id)), | |
| ...(ctx.defaultWebsiteId ? [ctx.defaultWebsiteId] : []), | |
| ...(ctx.websiteId ? [ctx.websiteId] : []), | |
| ]); | |
| if (websiteId && !allowedIds.has(websiteId)) { | |
| return { | |
| error: | |
| "Website is not in this workspace. Call list_websites to see available websites.", | |
| }; | |
| } | |
| const resolved = resolveToolWebsite(ctx, websiteId); |
| import { FaviconImage } from "@/components/analytics/favicon-image"; | ||
| import type { Website } from "@/hooks/use-websites"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { Button } from "@databuddy/ui"; |
There was a problem hiding this comment.
P2: This menu bypasses the dashboard DS menu boundary; implement it with components/ds DropdownMenu primitives instead of custom div/ul/li structure and @databuddy/ui controls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/components/agent/agent-mention-menu.tsx, line 6:
<comment>This menu bypasses the dashboard DS menu boundary; implement it with `components/ds` `DropdownMenu` primitives instead of custom `div/ul/li` structure and `@databuddy/ui` controls.</comment>
<file context>
@@ -0,0 +1,75 @@
+import { FaviconImage } from "@/components/analytics/favicon-image";
+import type { Website } from "@/hooks/use-websites";
+import { cn } from "@/lib/utils";
+import { Button } from "@databuddy/ui";
+
+interface AgentMentionMenuProps {
</file context>
Comment on lines
+113
to
+115
| const ctx = getAppContext(options); | ||
| let domain = params.domain; | ||
| if (!domain) { |
There was a problem hiding this comment.
P2: search_console now hard-requires experimental_context even when a domain is already provided. This can throw early and break callers that pass domain directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/tools/search-console.ts, line 113:
<comment>`search_console` now hard-requires `experimental_context` even when a domain is already provided. This can throw early and break callers that pass `domain` directly.</comment>
<file context>
@@ -95,13 +103,29 @@ export function createSearchConsoleTools(params: {
inputSchema: searchAnalyticsInput,
- execute: async (input) => {
+ execute: async (input, options) => {
+ const ctx = getAppContext(options);
+ let domain = params.domain;
+ if (!domain) {
</file context>
Suggested change
| const ctx = getAppContext(options); | |
| let domain = params.domain; | |
| if (!domain) { | |
| let domain = params.domain; | |
| if (!domain) { | |
| const ctx = getAppContext(options); |
| } | ||
|
|
||
| export function AgentPageClient({ chatId, websiteId }: AgentPageClientProps) { | ||
| const { activeOrganizationId } = useOrganizationsContext(); |
There was a problem hiding this comment.
P2: AgentPageClient renders the chat workspace before organization context is resolved, so it can pass null as organizationId and show a transient/wrong "No active workspace" state. Gate rendering until activeOrganizationId is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/app/(main)/websites/[id]/agent/_components/agent-page-client.tsx, line 13:
<comment>`AgentPageClient` renders the chat workspace before organization context is resolved, so it can pass `null` as `organizationId` and show a transient/wrong "No active workspace" state. Gate rendering until `activeOrganizationId` is available.</comment>
<file context>
@@ -1,16 +1,28 @@
}
export function AgentPageClient({ chatId, websiteId }: AgentPageClientProps) {
+ const { activeOrganizationId } = useOrganizationsContext();
+
return (
</file context>
| const { chatId, organizationId, hasWebsites, isLoading } = useGlobalAgent(); | ||
|
|
||
| useEffect(() => { | ||
| if (chatId) { |
There was a problem hiding this comment.
P2: Redirecting on chatId alone also fires for orgs with no websites. Gate the redirect on hasWebsites to avoid jumping to /agent/:chatId in the no-websites state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/app/(main)/agent/_components/global-agent-redirect.tsx, line 14:
<comment>Redirecting on `chatId` alone also fires for orgs with no websites. Gate the redirect on `hasWebsites` to avoid jumping to `/agent/:chatId` in the no-websites state.</comment>
<file context>
@@ -0,0 +1,29 @@
+ const { chatId, organizationId, hasWebsites, isLoading } = useGlobalAgent();
+
+ useEffect(() => {
+ if (chatId) {
+ router.replace(`/agent/${chatId}`);
+ }
</file context>
Comment on lines
+114
to
+120
| let domain = params.domain; | ||
| if (!domain) { | ||
| const resolved = resolveToolWebsite(ctx, input.websiteId); | ||
| domain = | ||
| resolved.domain || | ||
| (await getWebsiteDomain(resolved.websiteId)) || | ||
| undefined; |
There was a problem hiding this comment.
P2: search_console ignores websiteId whenever params.domain is present, so multi-website queries can silently return metrics for the wrong site.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/ai/tools/search-console.ts, line 114:
<comment>`search_console` ignores `websiteId` whenever `params.domain` is present, so multi-website queries can silently return metrics for the wrong site.</comment>
<file context>
@@ -95,13 +103,29 @@ export function createSearchConsoleTools(params: {
- execute: async (input) => {
+ execute: async (input, options) => {
+ const ctx = getAppContext(options);
+ let domain = params.domain;
+ if (!domain) {
+ const resolved = resolveToolWebsite(ctx, input.websiteId);
</file context>
Suggested change
| let domain = params.domain; | |
| if (!domain) { | |
| const resolved = resolveToolWebsite(ctx, input.websiteId); | |
| domain = | |
| resolved.domain || | |
| (await getWebsiteDomain(resolved.websiteId)) || | |
| undefined; | |
| const resolved = resolveToolWebsite(ctx, input.websiteId); | |
| let domain = | |
| resolved.domain || | |
| (resolved.websiteId === ctx.websiteId ? params.domain : undefined) || | |
| (await getWebsiteDomain(resolved.websiteId)) || | |
| undefined; |
| const router = useRouter(); | ||
| const { organizationId, hasWebsites, isLoading } = useGlobalAgent(); | ||
|
|
||
| if (organizationId && !(isLoading || hasWebsites)) { |
There was a problem hiding this comment.
P3: This repeats the same organizationId/hasWebsites/isLoading gating and skeleton UI already present in global-agent-redirect.tsx; consider extracting a shared guard/render helper to avoid logic drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/app/(main)/agent/_components/global-agent-page.tsx, line 14:
<comment>This repeats the same `organizationId/hasWebsites/isLoading` gating and skeleton UI already present in `global-agent-redirect.tsx`; consider extracting a shared guard/render helper to avoid logic drift.</comment>
<file context>
@@ -0,0 +1,40 @@
+ const router = useRouter();
+ const { organizationId, hasWebsites, isLoading } = useGlobalAgent();
+
+ if (organizationId && !(isLoading || hasWebsites)) {
+ return <AgentNoWebsites />;
+ }
</file context>
Add a global no-emoji rule to the shared behavior rules so it applies across the dashboard, MCP, and Slack variants, and drop the warning emoji from the link-delete confirmation the model could otherwise mirror.
Memoize each mention row and stabilize its callbacks so navigating with arrow keys or the pointer only re-renders the rows whose selection changed, switch hover from mouseEnter to mouseMove so a keyboard-driven scroll under a still cursor no longer hijacks the selection, and scroll the active row into view. Also lift the input toolbar into its own memoized component and the input surface into a named element so the menu wiring reads flat.
Make AgentWorkspace's new/select/delete handlers required props and remove the hidden route-param fallbacks from ChatHistory and NewChatButton, so the workspace no longer reads the route. Each page now owns its own navigation: the org page routes to /agent/* and the per-website page to /websites/[id]/agent/*, both clearing the stored last-chat key on delete-to-empty.
izadoesdev
changed the title
There was a problem hiding this comment.
0 issues found across 9 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 19 unresolved issues from previous reviews.
Re-trigger cubic
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
This branch carries two threads that accumulated on the feature branch ahead of
staging: a rebuild of the agent into an organization-scoped workspace, and a round of billing/auth hardening.Agent → organization workspace
organizationId(chatwebsiteIdis now nullable); list/get/rename/delete are org-scoped in RPCwebsiteIdand a newlist_websitestool lets the agent discover/compare sites@-mention to reference websites in the input; keep the per-website page (/websites/[id]/agent) auto-scoped via a pre-injected default websiteAgentWorkspaceno longer reads the route; each page owns its own routingAuth / session security
Billing / Autumn
Insights
resolveInsightsBillingDI seam that resolves the billing customer and checks agent credits before running the website agent; skip generation when credits are exhaustedsourceunion to include"insights"getComparisonPeriodwith the detectionwowWindowso the agent and signal detection share the same week-over-week windowChore
Test plan
bun run check-typesclean across all packagesbun test apps/insights/src/billing.test.ts(4 DB-free cases)bunx ultracite checkclean on touched files@-mention + multi-website queries in the agent/agent/*and/websites/[id]/agent/*Notes
websiteId+ org backfill was already applied; the org backfill was a no-op (all 140 existing chats already carryorganization_id).cacheTagstest failure (packages/rpc/src/utils/billing.test.ts) is pre-existing onstaging, order-dependent, and does not reproduce locally (full rpc suite: 227 pass / 0 fail).