Updated some stuff#66
Conversation
|
@Coder-soft is attempting to deploy a commit to the yamura3's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughSupabase authentication is bound to Wisp, identity metadata is extracted from session events, stored on machine records, and returned by dashboard queries. Documentation describes the integration, while an unrelated public test fixture gains one line of content. ChangesSupabase identity enrichment
Test fixture update
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Supabase
participant bindSupabase
participant Wisp
participant Convex
participant Dashboard
Supabase->>bindSupabase: auth state change
bindSupabase->>Wisp: identify or reset
Wisp->>Convex: session_identify event
Convex->>Convex: persist machine identity metadata
Dashboard->>Convex: query machine data
Convex-->>Dashboard: return identity fields
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
Greptile SummaryThis PR adds a small static test file under the public assets directory. The main change is:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| public/albums_test.txt | Adds a new public static text file with no observed conflict with the app's album data or asset paths. |
Reviews (1): Last reviewed commit: "Updated some stuff" | Re-trigger Greptile
- Update @renderdragonorg/wisp from 0.1.1 to 0.2.0 - Replace custom fetch transport with built-in wispSecret config option - Add bindSupabase() to auto-link auth state and enrich machine records with user email, name, and auth provider - Add userEmail, userName, authProvider fields to machines Convex schema - Handle session_identify events in analytics ingestion pipeline - Surface enriched user fields in dashboard queries (machine stats, page visitors, machine search) - Remove manual wisp.identify/reset from AuthProvider (now handled by bindSupabase)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@convex/dashboard.ts`:
- Around line 283-285: Restrict the new userEmail, userName, and authProvider
fields returned by getMachineStats, getPageVisitors, and searchMachines to
authenticated, tenant-scoped callers. Add the existing authorization and tenant
checks to these public query handlers, or remove/gate the identity fields so
unauthenticated callers cannot retrieve PII.
In `@convex/events.ts`:
- Around line 132-157: The recordBatch and corresponding event-processing
handler must scope extracted identity data to each machine/session instead of
reusing one batch-wide extractUserInfo result. Track identity state per target
while iterating events, update it for each session_identify event, and pass only
that target’s current identity to upsertMachine; add tests covering multiple
machines and identity changes within a batch.
In `@supabase.md`:
- Line 90: Update the diagram’s opening Markdown code fence at line 90 to
specify the text language, using the existing diagram content unchanged.
- Around line 25-27: Update the Supabase createClient example to read the
repository’s VITE_SUPABASE_PUBLISHABLE_KEY environment variable instead of
VITE_SUPABASE_ANON_KEY, while keeping the URL argument unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 86a4aa1b-7c12-423f-b7fc-31b867f99725
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
convex/dashboard.tsconvex/events.tsconvex/schema.tspackage.jsonpnpm-workspace.yamlsrc/main.tsxsrc/providers/AuthProvider.tsxsupabase.md
💤 Files with no reviewable changes (1)
- src/providers/AuthProvider.tsx
| userEmail: machine.userEmail ?? null, | ||
| userName: machine.userName ?? null, | ||
| authProvider: machine.authProvider ?? null, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Gate the new PII before returning it.
getMachineStats, getPageVisitors, and searchMachines now return email/name/provider, but the supplied dashboard handlers are public querys without an auth or tenant check. Anyone able to call these endpoints can retrieve user PII; add authorization and tenant scoping, or keep identity fields behind authenticated endpoints. (raw.githubusercontent.com)
Also applies to: 387-389, 442-444
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@convex/dashboard.ts` around lines 283 - 285, Restrict the new userEmail,
userName, and authProvider fields returned by getMachineStats, getPageVisitors,
and searchMachines to authenticated, tenant-scoped callers. Add the existing
authorization and tenant checks to these public query handlers, or remove/gate
the identity fields so unauthenticated callers cannot retrieve PII.
| function extractUserInfo(events: Array<{ name: string; payload?: unknown }>): { email?: string; name?: string; provider?: string } | undefined { | ||
| const identifyEvent = events.find((e) => e.name === "session_identify"); | ||
| if (!identifyEvent?.payload || typeof identifyEvent.payload !== "object") return undefined; | ||
| const p = identifyEvent.payload as Record<string, unknown>; | ||
| const email = typeof p.email === "string" ? p.email : undefined; | ||
| const name = typeof p.name === "string" ? p.name : undefined; | ||
| const provider = typeof p.provider === "string" ? p.provider : undefined; | ||
| return email || name || provider ? { email, name, provider } : undefined; | ||
| } | ||
|
|
||
| export const recordBatch = mutation({ | ||
| args: { events: v.array(eventValidator) }, | ||
| handler: async (ctx, { events }) => { | ||
| // Process oldest-first so ordering-dependent bookkeeping (session creation, counters) is correct. | ||
| const sorted = [...events].sort((a, b) => a.timestamp - b.timestamp); | ||
| const userInfo = extractUserInfo(sorted); | ||
|
|
||
| for (const event of sorted) { | ||
| const isBookkeeping = event.name === "session_start"; | ||
| const isBookkeeping = event.name === "session_start" || event.name === "session_identify"; | ||
|
|
||
| await upsertMachine(ctx, { | ||
| machineId: event.machineId, | ||
| userId: event.userId, | ||
| timestamp: event.timestamp, | ||
| meta: isBookkeeping ? event.payload : undefined, | ||
| userInfo, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Scope identity extraction to the machine/session being processed.
extractUserInfo(sorted) finds one session_identify event across the entire batch, then both handlers pass that object to every upsertMachine. A batch containing multiple machines or identity changes will assign the first user’s PII to unrelated machines and ignore later identities. Maintain identity state per target while processing events, and add multi-machine/multi-identity tests.
Also applies to: 233-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@convex/events.ts` around lines 132 - 157, The recordBatch and corresponding
event-processing handler must scope extracted identity data to each
machine/session instead of reusing one batch-wide extractUserInfo result. Track
identity state per target while iterating events, update it for each
session_identify event, and pass only that target’s current identity to
upsertMachine; add tests covering multiple machines and identity changes within
a batch.
| const supabase = createClient( | ||
| import.meta.env.VITE_SUPABASE_URL, | ||
| import.meta.env.VITE_SUPABASE_ANON_KEY |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the repository’s publishable-key environment variable.
The app’s Supabase client reads VITE_SUPABASE_PUBLISHABLE_KEY, not VITE_SUPABASE_ANON_KEY; following this example with the repository’s environment will pass the wrong value to createClient. (raw.githubusercontent.com)
Proposed fix
- import.meta.env.VITE_SUPABASE_ANON_KEY
+ import.meta.env.VITE_SUPABASE_PUBLISHABLE_KEY📝 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 supabase = createClient( | |
| import.meta.env.VITE_SUPABASE_URL, | |
| import.meta.env.VITE_SUPABASE_ANON_KEY | |
| const supabase = createClient( | |
| import.meta.env.VITE_SUPABASE_URL, | |
| import.meta.env.VITE_SUPABASE_PUBLISHABLE_KEY |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase.md` around lines 25 - 27, Update the Supabase createClient example
to read the repository’s VITE_SUPABASE_PUBLISHABLE_KEY environment variable
instead of VITE_SUPABASE_ANON_KEY, while keeping the URL argument unchanged.
|
|
||
| ## Data flow | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the diagram fence.
Change the opening fence at Line [90] to ```text so markdownlint MD040 passes.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase.md` at line 90, Update the diagram’s opening Markdown code fence at
line 90 to specify the text language, using the existing diagram content
unchanged.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit