Release staging to main#508
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 25 files
Confidence score: 3/5
- In
apps/api/src/integration/cache-auth-bypass.test.ts, the newflags.listdemo test skips the authenticated cache-prime step used by the rest of the cache-bypass suite, so it may pass while missing a real auth-bypass regression path — align it with the existing prime-then-unauthed-block pattern before merging. - In
apps/dashboard/app/(main)/websites/[id]/settings/general/page.tsx, rolling back the entire cached website onisPublictoggle failure can overwrite unrelated successful edits made during the pending request, causing user-visible state loss in the UI — limit rollback to the optimisticisPublicfield only. - In
apps/insights/src/delivery.ts, action labels are not going through the new implementation-detail filter, so code-like strings (for exampledocument./navigator.fragments) can leak into Slack-facing copy — route action labels through the same filter path before merge or gate with a targeted test. - In
apps/dashboard/app/(main)/websites/[id]/goals/_components/goal-item.tsx, wrappingGoalProgresswith a<span>that contains a block<div>produces invalid HTML that can cause subtle layout/accessibility inconsistencies — switch the wrapper to a<div>to de-risk markup issues.
Architecture diagram
sequenceDiagram
participant Visitor as Demo Visitor
participant Browser as Dashboard Browser
participant NavConfig as Nav Config
participant CS as Command Search
participant GoalsPage as Goals Page
participant FunnelsPage as Funnels Page
participant SettingsPage as Settings General
participant RPC as RPC Handler
participant DB as Database
participant Slack as Slack Delivery
Note over Visitor,Slack: Demo Website Navigation Flow
Visitor->>Browser: Navigate /demo/[id]/
Browser->>NavConfig: Read websiteNavigation config
NavConfig->>NavConfig: Filter items with hideFromDemo=true
alt hideFromDemo items
NavConfig-->>Browser: Exclude: Realtime, Anomalies, Users, Flags, Revenue, Agent, all Settings
else visible items
NavConfig-->>Browser: Show: Audience, Funnels, Goals, Pages, Events, etc.
end
Browser->>CS: Open command search on demo path
CS->>CS: Check isDemoPath flag
alt isDemoPath=true
CS->>CS: Skip "Actions", "Websites", "API Keys" categories
CS->>CS: Filter nav items by hideFromDemo
end
CS-->>Browser: Relevant search results only
Note over Visitor,Slack: Goals UI - Demo Read-Only Mode
Visitor->>Browser: Navigate /demo/[id]/goals
Browser->>GoalsPage: Detect isDemoRoute=true
alt isDemoRoute
GoalsPage->>GoalsPage: Hide "Create Goal" button
GoalsPage->>GoalsPage: Remove empty state create action
GoalsPage->>GoalsPage: Block Edit/Delete dialogs
end
Browser->>RPC: Fetch goals list
RPC-->>Browser: Goal data
Browser->>GoalsPage: Render read-only goal items
GoalsPage->>GoalsPage: Omit action menu (DropdownMenu)
Note over Visitor,Slack: Funnels UI - Demo Read-Only Mode
Browser->>FunnelsPage: Navigate /demo/[id]/funnels
FunnelsPage->>FunnelsPage: Detect isDemoRoute=true
alt isDemoRoute
FunnelsPage->>FunnelsPage: Hide "Create Funnel" button
FunnelsPage->>FunnelsPage: Remove empty state create action
FunnelsPage->>FunnelsPage: Block Edit/Delete dialogs
end
Browser->>RPC: Fetch funnels list (withPublicWorkspace allowed)
RPC-->>Browser: Funnel data
Browser->>FunnelsPage: Render read-only funnel items
FunnelsPage->>FunnelsPage: Omit action menu (DropdownMenu)
Note over Visitor,Slack: Feature Flags / Target Groups - RPC Auth
Visitor->>Browser: Attempt to view Flags or Target Groups in demo
Browser->>RPC: Call flags.list / targetGroups.list
RPC->>RPC: requireAuthedFlagRead OR requireAuthedTargetGroupRead
alt tier === "demo"
RPC-->>Browser: UNAUTHORIZED error
else tier === "authed"
RPC->>DB: Query with full rules/target groups
DB-->>RPC: Complete data
RPC-->>Browser: Full flag/target group data
end
Note over Visitor,Slack: Settings - Optimistic Public Toggle
Visitor->>SettingsPage: Toggle public visibility
SettingsPage->>SettingsPage: onMutate: Optimistic update
SettingsPage->>SettingsPage: Set QueryData for getWebsiteByIdKey
SettingsPage->>SettingsPage: Set QueryData for getWebsitesListKey
SettingsPage->>RPC: mutate websites.togglePublic
alt success
RPC-->>SettingsPage: Updated website
SettingsPage->>SettingsPage: updateWebsiteCache
SettingsPage->>SettingsPage: invalidateQueries
else error
RPC-->>SettingsPage: Error
SettingsPage->>SettingsPage: Rollback optimistic update
end
Note over Visitor,Slack: Shared Link - Canonical URL
SettingsPage->>SettingsPage: Compute shareableLink from publicConfig
SettingsPage-->>Visitor: Display canonical link, copy & open buttons
Note over Visitor,Slack: Slack Insight Digest
RPC->>DB: Generate insights with new schema fields
RPC->>RPC: Build digest with typed labels
RPC->>RPC: Strip IDs/implementation details from copy
RPC->>Slack: deliverInsightDigests with websiteName
Slack->>Slack: buildBlocks: Header "Insights for Name (domain)"
Slack->>Slack: buildBlocks: Label + Title + Evidence + Why it matters + Next
Slack->>Slack: buildFallbackText: Escaped plain text
Slack-->>Slack Channel: Post formatted blocks
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Greptile SummaryThis release hardens demo-tier access controls and improves the Slack insight digest format. Feature flag and target group definitions are now completely blocked for demo/unauthenticated callers (previously served with sanitized/empty rules), and all sensitive dashboard surfaces are hidden from demo navigation and command search.
Confidence Score: 4/5The auth hardening is sound and well-tested; demo callers now receive UNAUTHORIZED rather than stripped data, and the dashboard gating is backed by server-side auth. The security posture change is correct. A small number of non-blocking quality issues exist in delivery.ts — an aggressive try/catch filter and required-but-possibly-absent type/sentiment fields — neither causes wrong data to reach a user but both could produce noisier Slack fallbacks for existing insights. apps/insights/src/delivery.ts — the IMPLEMENTATION_DETAIL_MARKERS list and the required sentiment/type fields on DigestInsight deserve a second look before the next delivery iteration. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Demo/Public Caller
participant F as flags / target-groups Router
participant Auth as authorizeFlagRead
participant Guard as requireAuthedFlagRead
participant Cache as flagsCache
C->>F: "list({ websiteId })"
F->>Auth: authorizeFlagRead(context, input)
Auth-->>F: "workspace { tier: demo }"
F->>Guard: requireAuthedFlagRead(workspace)
Guard-->>C: UNAUTHORIZED
participant AC as Authed Caller
AC->>F: "list({ websiteId })"
F->>Auth: authorizeFlagRead(context, input)
Auth-->>F: "workspace { tier: authed }"
F->>Guard: requireAuthedFlagRead(workspace)
Guard-->>F: pass
F->>Cache: withCache(key, queryFn)
Cache-->>AC: flag definitions
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as Demo/Public Caller
participant F as flags / target-groups Router
participant Auth as authorizeFlagRead
participant Guard as requireAuthedFlagRead
participant Cache as flagsCache
C->>F: "list({ websiteId })"
F->>Auth: authorizeFlagRead(context, input)
Auth-->>F: "workspace { tier: demo }"
F->>Guard: requireAuthedFlagRead(workspace)
Guard-->>C: UNAUTHORIZED
participant AC as Authed Caller
AC->>F: "list({ websiteId })"
F->>Auth: authorizeFlagRead(context, input)
Auth-->>F: "workspace { tier: authed }"
F->>Guard: requireAuthedFlagRead(workspace)
Guard-->>F: pass
F->>Cache: withCache(key, queryFn)
Cache-->>AC: flag definitions
Reviews (1): Last reviewed commit: "Merge pull request #507 from databuddy-a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| function visibleSuggestion(value: string): string | null { | ||
| const copy = userVisibleCopy(value).trim(); | ||
| if (!copy) { | ||
| return null; | ||
| } | ||
|
|
||
| const lowerCopy = copy.toLowerCase(); | ||
| if ( | ||
| IMPLEMENTATION_DETAIL_MARKERS.some((marker) => lowerCopy.includes(marker)) | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
try/catch as an implementation-detail filter is overly broad
IMPLEMENTATION_DETAIL_MARKERS includes the string "try/catch", so any suggestion the AI writes containing "try/catch" — e.g. "Wrap the form submission in a try/catch block to handle network errors" — will be silently swapped out for a generic fallback via visibleSuggestion. This is a valid, human-readable action for an operator; it isn't a code implementation detail. The filter only applies when there are no actions[] labels, so it won't affect the typical case, but when actions are absent and the suggestion is human-readable, the wrong fallback will be shown. Consider removing "try/catch" from the list (or replacing it with .catch().
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| }); | ||
| } | ||
|
|
||
| function requireAuthedFlagRead(workspace: Workspace) { | ||
| if (workspace.tier === "demo") { | ||
| throw rpcError.unauthorized( | ||
| "Feature flag definitions require authenticated workspace access" | ||
| ); | ||
| } |
There was a problem hiding this comment.
byKey endpoint is now blocked for demo/public callers — confirm this is intentional for runtime SDK use
requireAuthedFlagRead is applied to list, byId, and byKey. If any client-side SDK uses byKey to evaluate flags at runtime for end-users of a public website, those callers now receive UNAUTHORIZED. The existing test only verifies flags.list is blocked, not byKey. If byKey is exclusively an admin-dashboard surface this is correct; if it is also used for runtime evaluation on public sites it would silently break flag delivery.
|
|
||
| function fallbackWhyItMatters(insight: DigestInsight): string { | ||
| switch (insight.type) { | ||
| case "referrer_change": | ||
| case "traffic_spike": | ||
| case "positive_trend": | ||
| return "This is a channel or segment worth repeating while the context is fresh."; | ||
| case "conversion_leak": |
There was a problem hiding this comment.
sentiment and type are required on DigestInsight but old DB rows may lack them
DigestInsight now declares both sentiment: string and type: string as non-optional. Insights generated before these fields were persisted will have undefined at runtime. digestLabel switches on insight.type so undefined hits the default branch safely, but sentiment is checked inside the referrer_change/traffic_spike/positive_trend branch, so those older insights will always show "Review · Traffic" even when they were originally positive. Marking both optional (sentiment?: string, type?: string) would match the runtime reality.
| const IMPLEMENTATION_DETAIL_MARKERS = [ | ||
| "document.", | ||
| "navigator.", | ||
| "execcommand", | ||
| "writetext", | ||
| "try/catch", | ||
| ] as const; |
There was a problem hiding this comment.
"try/catch" marker suppresses any human-readable suggestion that mentions error handling. Replacing it with .catch( targets only code syntax and avoids false positives.
| const IMPLEMENTATION_DETAIL_MARKERS = [ | |
| "document.", | |
| "navigator.", | |
| "execcommand", | |
| "writetext", | |
| "try/catch", | |
| ] as const; | |
| const IMPLEMENTATION_DETAIL_MARKERS = [ | |
| "document.", | |
| "navigator.", | |
| "execcommand", | |
| "writetext", | |
| ".catch(", | |
| ] as const; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
The latest updates on your projects. Learn more about Unkey Deploy
|
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Shadow auto-approve: would require human review. Large feature promotion with demo-mode, read-only surfaces, optimistic caching, Slack digest changes, and security hardening across multiple files.
Re-trigger cubic
1 participant
Summary
stagingtomain.stagingonto the latestmainso the release PR has a linear, neutral commit list.Validation
bunx ultracite checkbun run --cwd apps/insights testbun run --cwd apps/dashboard check-typesbun run check-typesdotenv -- turbo run testNotes