feat(frontend): Audit Log, Usage & Billing, and the /m write paths - #5892
feat(frontend): Audit Log, Usage & Billing, and the /m write paths#5892ardaerzin wants to merge 18 commits into
Conversation
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesShared settings platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
1880f06 to
e593c54
Compare
2d2afc2 to
f6bfe32
Compare
e593c54 to
034448b
Compare
f6bfe32 to
9ceda16
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
web/packages/agenta-settings-ui/src/audit/AuditLogTable.tsx (1)
141-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
useCallbackout of the JSX prop.The hook at line 149 runs inside a JSX attribute expression. The call is unconditional, so the hook order is safe, but the placement hides a hook from the top of the component and rebuilds the
filterselement on every render. Declare the callback with the other hooks and memoizefilters.♻️ Proposed refactor
const flushFiltersRef = useRef<() => void>(() => undefined) + const registerRefresh = useCallback((flush: () => void) => { + flushFiltersRef.current = flush + }, []) const handleReload = useCallback(() => { flushFiltersRef.current() refreshTable() }, [refreshTable]) - const filters: ReactNode = ( - <AuditLogFilters - registerRefresh={useCallback((flush: () => void) => { - flushFiltersRef.current = flush - }, [])} - renderDateRange={renderDateRange} - /> - ) + const filters: ReactNode = useMemo( + () => ( + <AuditLogFilters registerRefresh={registerRefresh} renderDateRange={renderDateRange} /> + ), + [registerRefresh, renderDateRange], + )web/oss/src/pages/w/[workspace_id]/p/[project_id]/settings/index.tsx (1)
80-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce these comments to one short line.
These comments describe normal code structure. They do not document a surprising constraint.
web/oss/src/pages/w/[workspace_id]/p/[project_id]/settings/index.tsx#L80-L86: replace the two layout comments with one short comment, or remove them.web/oss/src/lib/helpers/isEE.ts#L3-L6: replace the migration comment with one short comment, or remove it.As per coding guidelines, “Keep in-code comments to at most one short line.”
Source: Coding guidelines
web/mobile/src/features/settings/SettingsTabRail.tsx (1)
41-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose the selected tab to assistive technology.
The active tab is indicated by color and weight only. Add
aria-currentso screen readers announce the selection.♿ Proposed change
<button key={tab.key} type="button" + aria-current={tab.key === active ? "page" : undefined} onClick={() => onSelect(tab.key)}web/mobile/src/features/settings/settingsNavScope.tsx (1)
48-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated tab-select navigation in the settings rails. Both rails build the same shallow
router.replace({query: {...router.query, tab}})call, so the two paths can drift when routing changes.
web/mobile/src/features/settings/settingsNavScope.tsx#L48-L53: call a shareduseSelectSettingsTabhelper after theisSettingsTabKeyguard.web/mobile/src/features/settings/SettingsScreen.tsx#L323-L327: replace the localselectTabcallback with the same shared helper.web/mobile/src/features/settings/PlanChooserSheet.tsx (1)
78-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBottom-sheet classes are now duplicated in the primitive.
SheetContentappliesmax-h-[85vh],overflow-y-auto, androunded-t-2xlforside="bottom". These two sheets still pass their own copies with a different height cap, sotailwind-mergesilently overrides the shared cap to90vh.AccountTab.tsxalready dropped its copies in this PR.
web/mobile/src/features/settings/PlanChooserSheet.tsx#L78-L81: removemax-h-[90vh] overflow-y-auto rounded-t-2xland keep onlyside="bottom"plusgap-0.web/mobile/src/features/settings/CancelSubscriptionSheet.tsx#L53-L56: apply the same removal.web/mobile/src/components/ui/select.tsx (1)
46-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the popper-only CSS variables to
position="popper". Radix defines--radix-select-content-available-heightand--radix-select-content-transform-originonly for popper positioning. With the currentitem-aligneddefault, the height cap is invalid, so long lists can exceed the viewport. Either scope these utilities to the popper branch or defaultpositionto"popper".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: afa58a30-5de0-4aed-83c5-744cf1559da7
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (94)
web/ee/package.jsonweb/ee/src/components/SidebarBanners/state/atoms.tsweb/ee/src/components/pages/settings/AuditLog/AuditLog.tsxweb/ee/src/components/pages/settings/AuditLog/assets/constants.tsweb/ee/src/components/pages/settings/AuditLog/components/AuditEventDrawer.tsxweb/ee/src/components/pages/settings/AuditLog/components/AuditLogTable.tsxweb/ee/src/components/pages/settings/AuditLog/state.tsweb/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/AutoRenewalCancelModalContent/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/constants.tsweb/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/types.d.tsweb/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingCard/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingModalContent/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingModalTitle/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/SubscriptionPlanDetails/index.tsxweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/types.d.tsweb/ee/src/components/pages/settings/Billing/Modals/PricingModal/index.tsxweb/ee/src/components/pages/settings/Billing/assets/UsageProgressBar/index.tsxweb/ee/src/components/pages/settings/Billing/assets/types.d.tsweb/ee/src/components/pages/settings/Billing/index.tsxweb/mobile/src/components/ui/input.tsxweb/mobile/src/components/ui/select.tsxweb/mobile/src/components/ui/sheet.tsxweb/mobile/src/features/app/ContextSync.tsxweb/mobile/src/features/nav/AppShell.tsxweb/mobile/src/features/nav/NavDrawer.tsxweb/mobile/src/features/nav/NavRail.tsxweb/mobile/src/features/nav/lastNonSettingsPath.tsweb/mobile/src/features/nav/useMobileNavItems.tsxweb/mobile/src/features/settings/AccountTab.tsxweb/mobile/src/features/settings/BillingTab.tsxweb/mobile/src/features/settings/CancelSubscriptionSheet.tsxweb/mobile/src/features/settings/MembersTab.tsxweb/mobile/src/features/settings/PlanChooserSheet.tsxweb/mobile/src/features/settings/ProjectsTab.tsxweb/mobile/src/features/settings/SettingsScreen.tsxweb/mobile/src/features/settings/SettingsTabRail.tsxweb/mobile/src/features/settings/nestedNav.tsweb/mobile/src/features/settings/settingsNavScope.tsxweb/mobile/src/features/settings/settingsTabs.tsweb/mobile/src/lib/env.tsweb/mobile/tests/unit/nestedNav.test.tsweb/oss/src/components/DrillInView/OSSdrillInUIProvider.tsxweb/oss/src/components/Layout/Layout.tsxweb/oss/src/components/Sidebar/scopes/settingsScope.tsxweb/oss/src/components/pages/settings/Organization/UpgradePrompt.tsxweb/oss/src/components/pages/settings/WorkspaceManage/Modals/InviteUsersModal.tsxweb/oss/src/components/pages/settings/WorkspaceManage/WorkspaceManage.tsxweb/oss/src/components/pages/settings/hooks/useSettingsAccess.tsweb/oss/src/hooks/usePostAuthRedirect.tsweb/oss/src/hooks/useProjectPermissions.tsweb/oss/src/hooks/useWorkspacePermissions.tsweb/oss/src/lib/helpers/auth/turnstile.tsweb/oss/src/lib/helpers/isEE.tsweb/oss/src/lib/helpers/utils.tsweb/oss/src/pages/w/[workspace_id]/p/[project_id]/settings/index.tsxweb/oss/src/pages/workspaces/accept.tsxweb/oss/src/state/access/atoms.tsweb/packages/agenta-navigation-ui/src/NavMenu.tsxweb/packages/agenta-settings-ui/src/ApiKeysPage.tsxweb/packages/agenta-settings-ui/src/SettingsPageShell.tsxweb/packages/agenta-settings-ui/src/access/AccessUpgradeNotice.tsxweb/packages/agenta-settings-ui/src/access/SettingToggleRow.tsxweb/packages/agenta-settings-ui/src/access/entitlements.tsweb/packages/agenta-settings-ui/src/audit/AuditEventCells.tsxweb/packages/agenta-settings-ui/src/audit/AuditEventDrawer.tsxweb/packages/agenta-settings-ui/src/audit/AuditLogFilters.tsxweb/packages/agenta-settings-ui/src/audit/AuditLogPage.tsxweb/packages/agenta-settings-ui/src/audit/AuditLogTable.tsxweb/packages/agenta-settings-ui/src/audit/constants.tsweb/packages/agenta-settings-ui/src/billing/BillingPage.tsxweb/packages/agenta-settings-ui/src/billing/CancelSubscriptionReasons.tsxweb/packages/agenta-settings-ui/src/billing/PricingPlans.tsxweb/packages/agenta-settings-ui/src/billing/UsageProgressBar.tsxweb/packages/agenta-settings-ui/src/billing/api.tsweb/packages/agenta-settings-ui/src/billing/types.tsweb/packages/agenta-settings-ui/src/billing/useBillingCatalog.tsweb/packages/agenta-settings-ui/src/index.tsweb/packages/agenta-settings-ui/src/members/MembersPage.tsxweb/packages/agenta-settings-ui/src/organizations/OrganizationsPage.tsxweb/packages/agenta-settings-ui/src/projects/ProjectsPage.tsxweb/packages/agenta-settings-ui/src/secrets/NamedSecretTable.tsxweb/packages/agenta-settings-ui/src/secrets/SecretProviderTable.tsxweb/packages/agenta-settings-ui/src/tools/GatewayToolsSection.tsxweb/packages/agenta-settings-ui/src/triggers/TriggerConnectionsSection.tsxweb/packages/agenta-settings-ui/src/triggers/TriggerSchedulesSection.tsxweb/packages/agenta-settings-ui/src/triggers/TriggerSubscriptionsSection.tsxweb/packages/agenta-settings-ui/src/webhooks/WebhooksPage.tsxweb/packages/agenta-settings/package.jsonweb/packages/agenta-settings/src/index.tsweb/packages/agenta-settings/src/sidebar.tsxweb/packages/agenta-shared/src/api/env.tsweb/packages/agenta-shared/src/api/index.tsweb/packages/agenta-ui/src/components/ui/data-table.tsx
💤 Files with no reviewable changes (15)
- web/ee/src/components/pages/settings/AuditLog/state.ts
- web/ee/src/components/pages/settings/Billing/assets/types.d.ts
- web/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/types.d.ts
- web/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/constants.ts
- web/ee/src/components/pages/settings/AuditLog/components/AuditLogTable.tsx
- web/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingModalContent/index.tsx
- web/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingModalTitle/index.tsx
- web/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/types.d.ts
- web/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/SubscriptionPlanDetails/index.tsx
- web/ee/src/components/pages/settings/Billing/assets/UsageProgressBar/index.tsx
- web/ee/src/components/pages/settings/AuditLog/components/AuditEventDrawer.tsx
- web/ee/src/components/pages/settings/AuditLog/assets/constants.ts
- web/ee/src/components/pages/settings/Billing/Modals/AutoRenewalCancelModal/assets/AutoRenewalCancelModalContent/index.tsx
- web/ee/src/components/pages/settings/Billing/Modals/PricingModal/assets/PricingCard/index.tsx
- web/oss/src/components/Layout/Layout.tsx
| disabled: !selectOption || (selectOption == "something-else" && !inputOption), | ||
| disabled: !selectOption || (selectOption === CANCEL_REASON_OTHER && !inputOption), | ||
| }} | ||
| afterClose={() => setSelectOption("")} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset inputOption after close.
Line 59 resets selectOption but leaves inputOption unchanged. If the user selects CANCEL_REASON_OTHER, closes the modal, and reopens it, the old free-text reason remains visible.
Proposed fix
- afterClose={() => setSelectOption("")}
+ afterClose={() => {
+ setSelectOption("")
+ setInputOption("")
+ }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| afterClose={() => setSelectOption("")} | |
| afterClose={() => { | |
| setSelectOption("") | |
| setInputOption("") | |
| }} |
| const data = await checkoutNewSubscription({ | ||
| plan: plan.plan, | ||
| success_url: `${getEnv("NEXT_PUBLIC_AGENTA_WEB_URL")}${projectURL || ""}/settings?tab=billing`, | ||
| }) | ||
|
|
||
| window.open(data.data.checkout_url, "_blank") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep checkout navigation in the user action.
Line 70 opens the checkout window after checkoutNewSubscription() completes. Popup protection can block this window because the network await ends the trusted click context. Navigate the current tab to the checkout URL, or open a blank window before the await and assign its location after the response.
Proposed fix
- window.open(data.data.checkout_url, "_blank")
+ window.location.assign(data.data.checkout_url)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const data = await checkoutNewSubscription({ | |
| plan: plan.plan, | |
| success_url: `${getEnv("NEXT_PUBLIC_AGENTA_WEB_URL")}${projectURL || ""}/settings?tab=billing`, | |
| }) | |
| window.open(data.data.checkout_url, "_blank") | |
| const data = await checkoutNewSubscription({ | |
| plan: plan.plan, | |
| success_url: `${getEnv("NEXT_PUBLIC_AGENTA_WEB_URL")}${projectURL || ""}/settings?tab=billing`, | |
| }) | |
| window.location.assign(data.data.checkout_url) |
| const handleOpenPortal = async () => { | ||
| setOpeningPortal(true) | ||
| try { | ||
| const portalUrl = await openBillingPortal() | ||
| if (portalUrl) window.open(portalUrl, "_blank") | ||
| } finally { | ||
| setOpeningPortal(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
window.open runs after an await, so mobile browsers can block it. Both handlers fetch a Stripe URL first and open the tab afterwards. The call then lies outside the user-gesture window, and iOS Safari and Chrome for Android block it. The user taps and nothing happens.
web/mobile/src/features/settings/BillingTab.tsx#L52-L60: open the portal in the current tab withwindow.location.assign(portalUrl), or open a blank tab synchronously before the await and set itslocationafter it resolves.web/mobile/src/features/settings/PlanChooserSheet.tsx#L58-L63: apply the same handling to the checkout URL. If you navigate in place, the existingsuccessUrlalready returns the user to?tab=billing.
📍 Affects 2 files
web/mobile/src/features/settings/BillingTab.tsx#L52-L60(this comment)web/mobile/src/features/settings/PlanChooserSheet.tsx#L58-L63
| const canConfirm = Boolean(reason) && (reason !== CANCEL_REASON_OTHER || Boolean(otherReason)) | ||
|
|
||
| const confirm = async () => { | ||
| setError(null) | ||
| setCancelling(true) | ||
| try { | ||
| await cancelBillingSubscription(projectId) | ||
| onChanged() | ||
| onOpenChange(false) | ||
| setReason("") | ||
| setOtherReason("") | ||
| } catch { | ||
| setError("We couldn't cancel your subscription. Try again, or contact support.") | ||
| } finally { | ||
| setCancelling(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The collected cancellation reason is discarded.
canConfirm requires reason, and CANCEL_REASON_OTHER requires otherReason. confirm then calls cancelBillingSubscription(projectId) without either value. The user is forced to answer a question whose answer is dropped.
If cancelBillingSubscription accepts a reason payload, pass it. If it does not, remove the gating or add the payload to the shared API.
#!/bin/bash
# Check the signature of cancelBillingSubscription and how the desktop flow submits the reason.
rg -nP -C6 'cancelBillingSubscription' web/packages/agenta-settings-ui/src web/ee/src| const [pendingRemoval, setPendingRemoval] = useState<WorkspaceMember | null>(null) | ||
| const [error, setError] = useState<string | null>(null) | ||
|
|
||
| const canWrite = Boolean(organizationId && workspaceId) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
canWrite checks for identifiers, not for permission.
canWrite is true whenever organizationId and workspaceId exist. The desktop settings layer gates invite and remove through workspace permission hooks. As written, mobile shows both actions to users without the right, and the request fails only at the server.
Gate these props with the same permission source the desktop uses.
#!/bin/bash
# Compare the mobile gate with the desktop permission source.
rg -nP -C5 'canInviteMembers|canRemoveMembers' web --type=tsx --type=ts
cat -n web/oss/src/hooks/useWorkspacePermissions.tsAlso applies to: 114-115
|
|
||
| return ( | ||
| <Sheet open={open} onOpenChange={onOpenChange}> | ||
| <SheetContent className="w-[560px]"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the sheet width responsive.
w-[560px] is a fixed desktop width. This package is shared with the mobile surface, where a 560px panel overflows the viewport and forces horizontal scrolling. Cap the width on small screens.
♻️ Proposed fix
- <SheetContent className="w-[560px]">
+ <SheetContent className="w-full sm:w-[560px] sm:max-w-[560px]">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <SheetContent className="w-[560px]"> | |
| <SheetContent className="w-full sm:w-[560px] sm:max-w-[560px]"> |
| /** | ||
| * Audit Log — Settings Page | ||
| * | ||
| * Lists platform events from `POST /events/query` in a paginated table with a | ||
| * right-side detail drawer. | ||
| * | ||
| * Two-gate access model: | ||
| * - Tab VISIBILITY is a permission check (`view_events`), handled by the | ||
| * settings sidebar / page (`canViewEvents`). | ||
| * - Page CONTENT is gated by the audit entitlement: with it the table | ||
| * renders, without it an upgrade notice stands in its place. | ||
| * | ||
| * Both gates are the host's to resolve — this page only takes their answers. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reduce the new multi-line implementation comments.
Keep each in-code comment to one short line. Remove comments that only restate the code.
web/packages/agenta-settings-ui/src/audit/AuditLogPage.tsx#L1-L14: reduce the page implementation comment to a short purpose statement.web/packages/agenta-settings-ui/src/secrets/NamedSecretTable.tsx#L129-L130: replace the two-line explanation with one short comment or remove it.web/packages/agenta-settings-ui/src/secrets/SecretProviderTable.tsx#L38-L39: replace the two-line explanation with one short comment or remove it.
As per coding guidelines, “Keep in-code comments to at most one short line.”
📍 Affects 3 files
web/packages/agenta-settings-ui/src/audit/AuditLogPage.tsx#L1-L14(this comment)web/packages/agenta-settings-ui/src/secrets/NamedSecretTable.tsx#L129-L130web/packages/agenta-settings-ui/src/secrets/SecretProviderTable.tsx#L38-L39
Source: Coding guidelines
| import {axios, getAgentaApiUrl} from "@agenta/shared/api" | ||
|
|
||
| import type {BillingPlanOption, BillingUsage} from "./types" | ||
|
|
||
| /** Per-quota consumption for the current period. */ | ||
| export const fetchBillingUsage = async (projectId: string): Promise<BillingUsage> => { | ||
| const {data} = await axios.get(`${getAgentaApiUrl()}/billing/usage`, { | ||
| params: {project_id: projectId}, | ||
| }) | ||
| return data | ||
| } | ||
|
|
||
| /** The plans on offer — what the plan chooser lists. */ | ||
| export const fetchBillingPlans = async (projectId: string): Promise<BillingPlanOption[]> => { | ||
| const {data} = await axios.get(`${getAgentaApiUrl()}/billing/plans`, { | ||
| params: {project_id: projectId}, | ||
| }) | ||
| return Array.isArray(data) ? data : [] | ||
| } | ||
|
|
||
| /** Per-slug pricing metadata. Only read here for which slug is the free tier. */ | ||
| export const fetchBillingPricing = async (): Promise< | ||
| Record<string, {free?: boolean; trial?: number} | undefined> | ||
| > => { | ||
| const {data} = await axios.get(`${getAgentaApiUrl()}/billing/pricing`) | ||
| return data ?? {} | ||
| } | ||
|
|
||
| /** Paid → paid. Switching to the free tier goes through cancellation instead. */ | ||
| export const switchBillingPlan = async ({ | ||
| plan, | ||
| projectId, | ||
| }: { | ||
| plan: string | ||
| projectId: string | ||
| }): Promise<void> => { | ||
| await axios.post(`${getAgentaApiUrl()}/billing/plans/switch`, null, { | ||
| params: {plan, project_id: projectId}, | ||
| }) | ||
| } | ||
|
|
||
| /** Ends auto-renewal; the plan reverts to the free tier at the period boundary. */ | ||
| export const cancelBillingSubscription = async (projectId: string): Promise<void> => { | ||
| await axios.post(`${getAgentaApiUrl()}/billing/subscription/cancel`, null, { | ||
| params: {project_id: projectId}, | ||
| }) | ||
| } | ||
|
|
||
| /** Free (or no subscription) → paid. Returns the Stripe Checkout URL to send the payer to. */ | ||
| export const checkoutBillingSubscription = async ({ | ||
| plan, | ||
| successUrl, | ||
| }: { | ||
| plan: string | ||
| successUrl: string | ||
| }): Promise<string | null> => { | ||
| const {data} = await axios.post(`${getAgentaApiUrl()}/billing/stripe/checkouts/`, null, { | ||
| params: {plan, success_url: successUrl}, | ||
| }) | ||
| return data?.checkout_url ?? null | ||
| } | ||
|
|
||
| /** Returns the Stripe customer-portal URL — invoices, payment methods, cancellation. */ | ||
| export const openBillingPortal = async (): Promise<string | null> => { | ||
| const {data} = await axios.post(`${getAgentaApiUrl()}/billing/stripe/portals/`) | ||
| return data?.portal_url ?? null |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Replace raw axios calls with validated Fern resource accessors.
These modules bypass the required frontend API boundary. Raw axios calls use {params: ...} and return unchecked response data. This can accept an incompatible backend payload and break billing or entitlement rendering.
web/packages/agenta-settings-ui/src/billing/api.ts#L6-L71: Add or use per-resource Fern accessors. Pass query values through{queryParams: {...}}. Validate each response withsafeParseWithLogging.web/packages/agenta-settings-ui/src/access/entitlements.ts#L14-L66: Use the same validated Fern accessors for plans and subscriptions. Do not expose unvalidated response data to entitlement calculation.
As per coding guidelines, “Frontend API code must use per-resource Fern client accessors” and “Keep Zod validation at API boundaries using safeParseWithLogging.”
#!/bin/bash
set -euo pipefail
ast-grep outline web/packages/agenta-settings-ui/src/billing/api.ts --items all
ast-grep outline web/packages/agenta-settings-ui/src/access/entitlements.ts --items all
rg -n -C 3 'billing|subscription|access.*plan|queryParams|safeParseWithLogging' \
web/packages/agenta-sdk/src/resources.ts \
web/packages/agenta-settings-ui/src \
web/packages/agenta-entities/src 2>/dev/null || true
fd package.json web/packages/agenta-settings-ui -x sh -c 'echo "--- $1"; cat "$1"' sh {}📍 Affects 2 files
web/packages/agenta-settings-ui/src/billing/api.ts#L6-L71(this comment)web/packages/agenta-settings-ui/src/access/entitlements.ts#L14-L66
| const formatPrice = (plan: BillingPlanOption) => { | ||
| if (!plan.price) return "Contact us" | ||
| const prefix = plan.price.base?.starting_at ? "Starts at " : "" | ||
| return `${prefix}$${plan.price.base?.amount} /month` | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against a missing base amount.
plan.price can exist without base. The optional chaining then yields undefined, and the card renders $undefined /month. Fall back to the contact copy when no amount is present.
🐛 Proposed fix
const formatPrice = (plan: BillingPlanOption) => {
- if (!plan.price) return "Contact us"
- const prefix = plan.price.base?.starting_at ? "Starts at " : ""
- return `${prefix}$${plan.price.base?.amount} /month`
+ const base = plan.price?.base
+ if (base?.amount == null) return "Contact us"
+ const prefix = base.starting_at ? "Starts at " : ""
+ return `${prefix}$${base.amount} /month`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const formatPrice = (plan: BillingPlanOption) => { | |
| if (!plan.price) return "Contact us" | |
| const prefix = plan.price.base?.starting_at ? "Starts at " : "" | |
| return `${prefix}$${plan.price.base?.amount} /month` | |
| } | |
| const formatPrice = (plan: BillingPlanOption) => { | |
| const base = plan.price?.base | |
| if (base?.amount == null) return "Contact us" | |
| const prefix = base.starting_at ? "Starts at " : "" | |
| return `${prefix}$${base.amount} /month` | |
| } |
| {!isUnlimited && value >= limit ? ( | ||
| <WarningIcon weight="fill" className="text-colorWarning" /> | ||
| ) : null} | ||
| </span> | ||
|
|
||
| <span className="flex items-center gap-2"> | ||
| <span className="text-xs font-medium text-colorText">{`${value} / ${limit ? limit : "-"}`}</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not warn when the limit is zero or unknown.
If limit is 0, value >= limit is true and isUnlimited is false, so the warning icon shows while line 63 renders the limit as "-". BillingPage reaches this state for the Free members bar whenever users.free is 0. Require a positive limit before you show the warning.
🐛 Proposed fix
- {!isUnlimited && value >= limit ? (
+ {!isUnlimited && limit > 0 && value >= limit ? (
<WarningIcon weight="fill" className="text-colorWarning" />
) : null}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!isUnlimited && value >= limit ? ( | |
| <WarningIcon weight="fill" className="text-colorWarning" /> | |
| ) : null} | |
| </span> | |
| <span className="flex items-center gap-2"> | |
| <span className="text-xs font-medium text-colorText">{`${value} / ${limit ? limit : "-"}`}</span> | |
| {!isUnlimited && limit > 0 && value >= limit ? ( | |
| <WarningIcon weight="fill" className="text-colorWarning" /> | |
| ) : null} | |
| </span> | |
| <span className="flex items-center gap-2"> | |
| <span className="text-xs font-medium text-colorText">{`${value} / ${limit ? limit : "-"}`}</span> |
First write affordance on mobile settings. ProjectsTab supplies the three surfaces as bottom sheets, mirroring AccountTab; the mutations stay in ProjectsPage, so this only collects the input and the desktop's antd modals are untouched. Adds the shadcn input from the registry, which mobile did not have — AccountTab's only field is owned by the shared page, so nothing had needed one yet. It came in with upstream's formatting and needed a prettier pass to match the repo. Rename seeds from the row's current name until the field is edited: the sheet mounts before a row is chosen, so it cannot take a defaultValue. Gates: lint 24/24, tsc 0 across mobile, oss and ee.
MembersTab supplies invite and remove as bottom sheets, same shape as ProjectsTab. Invite fetches the workspace roles only when the sheet opens, and sends without a role when the list comes back empty — the API treats roles as optional. Both write verbs stay off until an organization and workspace resolve, so the buttons never appear in a state where the call would 400. Role editing is deliberately left on the desktop. It is a per-row select, and a dropdown inside a table row is a poor trade on a phone; it deserves its own surface rather than a cramped port. Adds the shadcn select, formatted and import-grouped to match the repo. Gates: lint 24/24, tsc 0 across mobile, oss and ee.
Moves the seven EE Audit Log files into @agenta/settings-ui as AuditLogPage, a view fed by props: the audit entitlement, the date-range control and the upgrade link all arrive from the host. EE keeps a thin binding at the old path that supplies all three; /m renders the same page with none of them. The table is DataTable rather than InfiniteVirtualTable — antd-free, so it can render on mobile — which trades scroll-driven paging for an explicit "Load more" and hands the page back its own scroll (the full-height layout carve-out for ?tab=auditLog goes with it). antd's Descriptions, Spin, Tag, Tooltip, Skeleton and Empty become a definition list, Spinner, Tag, title attributes, DataTable's own loading state and EmptyState. The actor resolves through UserAuthorLabel from @agenta/entities directly rather than OSS's UserReference wrapper, and falls back to the raw user id so a departed member still identifies. Drawer state is local to the page instead of module atoms, and the drawer itself is Sheet.
…gs-ui, off antd Splits the EE Billing page into views and host. @agenta/settings-ui gains BillingPage (the plan card, the quota grid, members, the portal entry), PricingPlans and CancelSubscriptionReasons — the bodies of the two dialogs — plus UsageProgressBar and structural billing types of its own, so nothing in the package reaches for the OSS service layer. Each verb is a prop, and the control for a verb the host does not pass hides itself: a read-only surface reports the plan and the usage and offers no way to change either. EE keeps everything that talks to Stripe: the subscription and usage queries, the checkout / switch / cancel calls, the portal, the return-from-Stripe and ?upgrade=true round-trips, and the two EnhancedModal shells. The one read a host with no billing layer needs — /billing/usage — moves into the package. antd's Spin, Typography, Button, Card, Radio, Input and Space become Spinner, plain elements, @agenta/ui Button, a bordered panel, RadioGroup and Input; @ant-design/icons' WarningFilled becomes a filled phosphor Warning. The raw --ag-c-* hex tokens the page carried become semantic ones, so it now renders correctly in dark mode.
Access & Security crashed on /m — the mobile app has no global TooltipProvider, and this was the one tooltip in the package not carrying its own. Every other section already wraps one.
Adds useEntitlements: the access-controls catalog from /access/plans, keyed by the plan slug on /billing/subscription, giving the same six flags the desktop gates on. For a host with no entitlement layer of its own — the mobile app was rendering Access Controls, Verified Domains, SSO and the Audit Log unconditionally, so a plan that includes none of them still showed every control. The desktop keeps its own jotai version: those atoms are also gated behind its idle-boot pass and share the subscription query with the billing UI, neither of which a second caller should inherit.
Gating each of Access Controls, Verified Domains and SSO Providers separately meant a plan that includes none of them rendered three identical lock cards down the page, each saying the same thing. AccessUpgradeNotice takes whichever features are locked and says it once, naming them — and titles itself for the whole page when all three are out.
Two bugs on the same card. The plan name came from a fixed slug segment
(plan.split("_")[2]), which is empty for any slug that is not namespaced three deep —
and slugs are env-overridable, so plenty are not. It now takes the last segment, which
reads both shapes, and falls back to an em-dash rather than nothing.
The renewal line ran dayjs.unix() on a period_end the backend leaves unset for plans
that never renew, printing "Auto renews on Invalid Date". It now renders only against
a boundary that actually parses, and the type says period_end is optional because it is.
The card itself no longer requires billing to be switched on: a host that can only read
a subscription (the mobile app) still names the plan its quotas belong to.
…ta/shared processEnv is the list of keys Next inlines at build time; the billing flag was missing from it, so any package-layer read resolved only through the container's __env.js and came back empty everywhere else.
…lling service useBillingSubscription is now the single reader, shared by the entitlement gates and by whoever names the plan, so the two cannot drift apart or race for the same cache entry. It carries the desktop's retry policy, which was missing here: never retry a 4xx, and never retry 502/503/504. A billing service that is down answers every request that way, and the default three attempts just queue slow failures behind render-critical traffic.
…genta/settings-ui The package could read usage and the subscription but not act on either, so a host with no billing layer could only report a plan, never change one. Adds the four writes behind the page — switch, cancel, Stripe checkout, Stripe portal — and the two catalog reads. useBillingCatalog derives the pair the page cannot guess: which slug is the free tier (the pricing map says, and slugs are env-overridable) and whether the current plan is contact-sales. Without them the chooser offers to "upgrade" to the free plan and sends a downgrade through checkout instead of cancellation.
Triggers is missing from EE settings because it is gated on NEXT_PUBLIC_AGENTA_TOOLS_ENABLED, which is declared nowhere in hosting or CI — so it is undefined everywhere and both Tools and Triggers stay hidden. Nothing regressed; the flag has simply never been set. (One flag drives both tabs, which is worth revisiting: Triggers has little to do with the tool catalog.) /m had these hardcoded true, so it showed both tabs while the desktop hid them — the opposite of aligning the two surfaces. It also tested `license === "ee"`, missing the `cloud*` tiers the desktop's isEE() accepts. The four env-only gates move to @agenta/shared/api, which both hosts already import, and oss/lib/helpers/isEE.ts keeps only isEmailAuthEnabled — that one reads resolved auth config, not a bare env var. 15 call sites repointed. Mobile's build-time env allowlist gains the license, tools and billing keys. Without them a built image reads "" no matter how the container is configured, because `process.env[key]` with a computed key is not inlined. Gates: lint 24/24, tsc 0 across shared, oss, ee and mobile.
A `window.open` issued after awaiting Stripe sits outside the user-gesture window, so iOS Safari and Chrome for Android block it: the user taps Manage billing or a plan and nothing happens. The desktop pricing modal has the same shape. Reserve the tab synchronously in the handler and point it at the URL once the request resolves; close it if the request fails or returns nothing. The handle rules out `noopener`, so the back-reference is severed by hand. One helper in @agenta/settings-ui, so the mobile and desktop paths cannot diverge; a blocked popup still falls back to navigating in place.
`canWrite = Boolean(organizationId && workspaceId)` is true for every signed-in member on a real route, so invite and remove rendered for viewers without the right and the request only failed at the server. The desktop's rule lives in web/oss/src/hooks/useWorkspacePermissions, built on oss-only jotai stores that no other host can import. Extract the rule itself into @agenta/settings-ui as a pure function over the roster the members page already holds, and read it from /m. Unlike the desktop, an unresolved answer is a no rather than the not-enforced yes — the entitlement is now fetched for the workspace tab so RBAC is known before anything is offered. The project-role narrowing the desktop applies is not reproduced: /m does not load `project.user_role`, so the union of the member's role permissions stands in for it. oss's hook should be refactored onto this function rather than a third copy being written.
`router.query` is empty on the first client render of a statically optimized page, so a direct load or refresh of `?tab=billing` resolved to Preferences, rendered it, and started its queries before correcting itself. Gating the read on `router.isReady` alone does not fix it — it yields the same Preferences fallback on that first frame. The tab is nullable instead, and the page holds its body until one resolves, the way the other /m screens guard on readiness.
Three fields that did not mean what they showed. /m's cancellation sheet required a reason and then dropped it: `POST /billing/subscription/cancel` takes nothing but the project, and no client path carries the answer anywhere. Rather than gate a destructive action on a question with no reader, /m stops asking. The desktop keeps its questionnaire and its TODO to wire one up. /m's rename fell back to the project's current name when the field was emptied, so clearing it and submitting reported success and changed nothing. The draft is nullable now — untouched is seeded from the row, cleared stays cleared, and submit is blocked with the reason shown. The desktop's cancel modal reset its select on close but not its free-text box, so reopening still showed the previous reason.
The audit drawer pinned itself to 560px through a class, but `cn` is a plain join — that raced the sheet variant's own width instead of replacing it. Set it inline the way EnhancedDrawer does, capped at the viewport so a phone does not scroll sideways. A plan priced without a `base` rendered "$undefined /month"; it reads as priced on request now. A quota whose limit is 0 shows "-" for the limit yet still raised the over-quota warning, which the Free members bar hits. Comments trimmed to a line where they only restated the code.
DataTable bound onReload straight to onClick, so every caller's zero-arg reloader was handed the click event — the same trap the secret tables had before the reload moved into the shell. SettingToggleRow went back to a hover-only span mid-refactor; SimpleTooltip brings its own provider and the button trigger opens on focus and closes on Escape.
034448b to
4832ba1
Compare
9ceda16 to
6f9fe71
Compare
The settings-nav plan described a takeover that removes the in-page top bar unconditionally. Below lg the sidebar IS a drawer and that bar holds the only trigger for it, so taken literally the plan strands a phone user on whichever settings tab they landed on with no way back to the nav. "No top bar" is now scoped to lg and up; below it a trigger-only header survives, carrying the settings scope so the drawer opens the settings nav rather than the app nav. That is also what the shipped SettingsScreen does, so the plan and the code now agree. Added a status block: steps 1-4 are already implemented on pkg/settings-ee-pages (#5892) and what remains is the browser pass — and the phone breakpoint is the case to look at first. sidebar.ts is sidebar.tsx; the icon map holds JSX. restack-onto-112 reads as live instructions but describes the world before the stack existed: 88 commits, no lane branches, no PRs, 17 mobile type errors. Running it today would rebuild lanes that already have open PRs on a release branch other people are assembling. It now opens with a superseded banner pointing at execute-stacked-prs, and says what is still worth reading in it. plan.md counted its own lanes wrong (18, for T1-T9 plus L1-L10) and claimed zero double-placement over selectors that visibly overlap between tiers. The overlap is real and fine — Tier 1 carves the committed tree delta, Tier 2 the uncommitted paths on top of it, so a file legitimately appears in both — but the doc never said so. Within a tier is where it matters, and that is what the claim now says. The tier totals do not reconcile (274 against 261 and 346, counted at different moments by different rules) so the doc no longer quotes them; the tree-equality check is the real verification and holds either way. Section 7's two renamed branches are named. execute-stacked-prs guessed these docs would go on the bottom lane. They went on their own lane at the top, #5894, which is the better placement: a docs-only lane touches no code file and cannot conflict with anything below it. Its PR table is also now marked as counts *as opened* — review fixes land on the lanes afterwards, so live counts drift above it, and that is the cascade working rather than a broken stack.
The settings-nav plan described a takeover that removes the in-page top bar unconditionally. Below lg the sidebar IS a drawer and that bar holds the only trigger for it, so taken literally the plan strands a phone user on whichever settings tab they landed on with no way back to the nav. "No top bar" is now scoped to lg and up; below it a trigger-only header survives, carrying the settings scope so the drawer opens the settings nav rather than the app nav. That is also what the shipped SettingsScreen does, so the plan and the code now agree. Added a status block: steps 1-4 are already implemented on pkg/settings-ee-pages (#5892) and what remains is the browser pass — and the phone breakpoint is the case to look at first. sidebar.ts is sidebar.tsx; the icon map holds JSX. restack-onto-112 reads as live instructions but describes the world before the stack existed: 88 commits, no lane branches, no PRs, 17 mobile type errors. Running it today would rebuild lanes that already have open PRs on a release branch other people are assembling. It now opens with a superseded banner pointing at execute-stacked-prs, and says what is still worth reading in it. plan.md counted its own lanes wrong (18, for T1-T9 plus L1-L10) and claimed zero double-placement over selectors that visibly overlap between tiers. The overlap is real and fine — Tier 1 carves the committed tree delta, Tier 2 the uncommitted paths on top of it, so a file legitimately appears in both — but the doc never said so. Within a tier is where it matters, and that is what the claim now says. The tier totals do not reconcile (274 against 261 and 346, counted at different moments by different rules) so the doc no longer quotes them; the tree-equality check is the real verification and holds either way. Section 7's two renamed branches are named. execute-stacked-prs guessed these docs would go on the bottom lane. They went on their own lane at the top, #5894, which is the better placement: a docs-only lane touches no code file and cannot conflict with anything below it. Its PR table is also now marked as counts *as opened* — review fixes land on the lanes afterwards, so live counts drift above it, and that is the cascade working rather than a broken stack.
The EE-only settings pages come across: Audit Log and the Usage & Billing views move into
@agenta/settings-uiand off antd, and plan entitlements become resolvable from the package./mgains its first write paths — create, rename and delete projects; invite and remove members.Those call real mutation endpoints and have not been exercised in a browser.
Fixes that came out of doing it: one locked panel for Access & Security instead of three, the plan
card names the plan and no longer renders
Invalid Date,NEXT_PUBLIC_AGENTA_BILLING_ENABLEDisreadable from
@agenta/shared,SettingToggleRowbrings its ownTooltipProvider, and there isnow one subscription query that stops retrying a billing service that is failing rather than
hammering it.
Not run in a browser — static gates only (
pnpm lint-fix24/24,tsc --noEmitcleanfor
@agenta/shared,ui,entities,entity-ui,settings-ui,oss,ee,mobile).Stacked on
pkg/entity-ui-form-engine; review only this lane's diff.