Resolve conflicts: Deno types, safe client creation, and health/brain/sync scripts#97
Conversation
…h/brain/sync:check scripts Co-authored-by: asperpharma <252395498+asperpharma@users.noreply.github.com>
Signed-off-by: Asper Beauty Shop <252395498+asperpharma@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Pull request overview
This PR resolves conflicts from PR #88 by reapplying the valid changes on top of main, focusing on Supabase Edge Function robustness and adding local connectivity/diagnostic scripts.
Changes:
- Adjusted
beauty-assistantEdge Function to avoid unsafe env var non-null assertions for optional clients and to suppress TS tooling issues with Deno URL imports. - Added
health,brain, andsync:checknpm scripts plus new Node-based connectivity check scripts. - Expanded README with connectivity script docs and an “Available Scripts” table.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/functions/beauty-assistant/index.ts | Adds Deno/URL-import TS workarounds; updates Supabase client creation for product context and telemetry attribution. |
| scripts/brain-check.js | New CLI script to validate Beauty Assistant reachability via GET. |
| scripts/sync-check.js | New CLI script to validate frontend /health and Beauty Assistant concurrently. |
| package.json | Registers new health, brain, and sync:check scripts. |
| package-lock.json | Lockfile update for dependency graph changes. |
| README.md | Documents the new scripts and adds an “Available Scripts” table. |
| const BRAIN_URL = | ||
| "https://qqceibvalkoytafynwoc.supabase.co/functions/v1/beauty-assistant"; | ||
|
|
There was a problem hiding this comment.
This script hardcodes the Supabase project URL. Other repo scripts support environment overrides (e.g. SITE_URL, VITE_SUPABASE_URL) which makes local/staging checks easier and avoids needing code edits when the project changes. Consider building BRAIN_URL from process.env.VITE_SUPABASE_URL (or SUPABASE_URL) with this value as the default.
| const BRAIN_URL = | |
| "https://qqceibvalkoytafynwoc.supabase.co/functions/v1/beauty-assistant"; | |
| const SUPABASE_URL = | |
| process.env.VITE_SUPABASE_URL || | |
| process.env.SUPABASE_URL || | |
| "https://qqceibvalkoytafynwoc.supabase.co"; | |
| const BRAIN_URL = `${SUPABASE_URL.replace(/\/+$/, "")}/functions/v1/beauty-assistant`; |
|
|
||
| async function main() { | ||
| console.log("Asper Beauty Shop — Brain connectivity check\n"); | ||
| try { | ||
| const res = await fetch(BRAIN_URL, { method: "GET" }); | ||
| const text = await res.text(); |
There was a problem hiding this comment.
fetch() here has no timeout, so the process can hang indefinitely on stalled connections/DNS issues. Other diagnostic scripts in this repo implement a request timeout (see scripts/health-check.js / scripts/test-brain.js). Consider adding an AbortController timeout (or reusing the existing fetchUrl helper pattern) for more reliable CI/local runs.
| async function main() { | |
| console.log("Asper Beauty Shop — Brain connectivity check\n"); | |
| try { | |
| const res = await fetch(BRAIN_URL, { method: "GET" }); | |
| const text = await res.text(); | |
| const REQUEST_TIMEOUT_MS = 15000; | |
| async function main() { | |
| console.log("Asper Beauty Shop — Brain connectivity check\n"); | |
| try { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); | |
| let res; | |
| let text; | |
| try { | |
| res = await fetch(BRAIN_URL, { method: "GET", signal: controller.signal }); | |
| text = await res.text(); | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } |
| const FRONTEND_HEALTH = "https://asperbeautyshop-com.lovable.app/health"; | ||
| const BRAIN_URL = | ||
| "https://qqceibvalkoytafynwoc.supabase.co/functions/v1/beauty-assistant"; |
There was a problem hiding this comment.
These endpoints are hardcoded, which makes the script less reusable for staging/local environments. To match the pattern used by scripts/health-check.js and scripts/test-brain.js, consider reading the base URLs from env vars (e.g. SITE_URL / VITE_SUPABASE_URL) and deriving /health and the functions URL from those, with the current values as defaults.
| const FRONTEND_HEALTH = "https://asperbeautyshop-com.lovable.app/health"; | |
| const BRAIN_URL = | |
| "https://qqceibvalkoytafynwoc.supabase.co/functions/v1/beauty-assistant"; | |
| const SITE_URL = | |
| process.env.SITE_URL || "https://asperbeautyshop-com.lovable.app"; | |
| const SUPABASE_URL = | |
| process.env.VITE_SUPABASE_URL || "https://qqceibvalkoytafynwoc.supabase.co"; | |
| const FRONTEND_HEALTH = SITE_URL.replace(/\/+$/, "") + "/health"; | |
| const BRAIN_URL = | |
| SUPABASE_URL.replace(/\/+$/, "") + "/functions/v1/beauty-assistant"; |
| async function fetchStatus(name, url) { | ||
| try { | ||
| const res = await fetch(url, { method: "GET" }); | ||
| return { name, status: res.status, ok: res.ok }; | ||
| } catch (err) { | ||
| return { name, status: null, ok: false, error: err.message }; |
There was a problem hiding this comment.
Like brain-check.js, this uses fetch() without any timeout/abort handling. A stalled request can keep the script running forever, which is especially painful when used as a pre-deploy check. Consider adding an AbortController timeout around each request (or sharing a helper) similar to the existing timeout-based approach in scripts/health-check.js.
| async function fetchStatus(name, url) { | |
| try { | |
| const res = await fetch(url, { method: "GET" }); | |
| return { name, status: res.status, ok: res.ok }; | |
| } catch (err) { | |
| return { name, status: null, ok: false, error: err.message }; | |
| async function fetchStatus(name, url, timeoutMs = 10000) { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => { | |
| controller.abort(); | |
| }, timeoutMs); | |
| try { | |
| const res = await fetch(url, { method: "GET", signal: controller.signal }); | |
| return { name, status: res.status, ok: res.ok }; | |
| } catch (err) { | |
| const message = | |
| err && err.name === "AbortError" | |
| ? `Request timed out after ${timeoutMs}ms` | |
| : err && err.message | |
| ? err.message | |
| : "Unknown error"; | |
| return { name, status: null, ok: false, error: message }; | |
| } finally { | |
| clearTimeout(timeoutId); |
| const adminKey = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY") ?? Deno.env.get("SUPABASE_ANON_KEY"); | ||
| const adminClient = adminUrl && adminKey ? createClient(adminUrl, adminKey) : supabaseClient; |
There was a problem hiding this comment.
telemetry_events INSERT is protected by RLS to authenticated users (policy requires auth.uid() = user_id). When SUPABASE_SERVICE_ROLE_KEY is unset, this code creates a client with the anon key but without the user's Authorization header, so the insert will consistently fail (auth.uid will be null). Prefer falling back to the already-authenticated supabaseClient (or pass global.headers.Authorization: authHeader when creating the fallback client) and only use a separate client when the service role key is present.
| const adminKey = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY") ?? Deno.env.get("SUPABASE_ANON_KEY"); | |
| const adminClient = adminUrl && adminKey ? createClient(adminUrl, adminKey) : supabaseClient; | |
| const serviceRoleKey = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY"); | |
| const adminClient = adminUrl && serviceRoleKey | |
| ? createClient(adminUrl, serviceRoleKey) | |
| : supabaseClient; |
| // @ts-expect-error — Deno URL imports; resolved at runtime by Supabase Edge | ||
| import { serve } from "https://deno.land/std@0.168.0/http/server.ts"; | ||
| // @ts-expect-error — Deno URL imports; resolved at runtime by Supabase Edge |
There was a problem hiding this comment.
Using // @ts-expect-error here will start failing the moment URL imports become type-valid (the directive itself becomes an error). Since this is an environment-compatibility workaround, consider @ts-ignore (or an eslint/tsconfig setting) so future toolchain updates don’t break builds unexpectedly.
| // @ts-expect-error — Deno URL imports; resolved at runtime by Supabase Edge | |
| import { serve } from "https://deno.land/std@0.168.0/http/server.ts"; | |
| // @ts-expect-error — Deno URL imports; resolved at runtime by Supabase Edge | |
| // @ts-ignore — Deno URL imports; resolved at runtime by Supabase Edge | |
| import { serve } from "https://deno.land/std@0.168.0/http/server.ts"; | |
| // @ts-ignore — Deno URL imports; resolved at runtime by Supabase Edge |
PR #88 (
2026-03-04-lzdy) diverged frommainacross four files. This applies the valid changes from that branch cleanly on top of currentmain.supabase/functions/beauty-assistant/index.tsdeclare const Denoand@ts-expect-erroron the Deno URL imports to fix TypeScript compilation!) with conditional client creation in two places in the website chat path — matching the pattern already used in the webhook path:Same conditional pattern applied to the
adminClientin thecampaignSourcetelemetry block.package.jsonAdded three npm scripts; existing
sync/sync:dry/sync:publish(Shopify catalog) are untouched:healthnode scripts/health-check.jsbrainnode scripts/brain-check.jssync:checknode scripts/sync-check.jsNew scripts
scripts/brain-check.js— hits the Beauty Assistant edge function; treats only 2xx as success (401/405 surface misconfiguration)scripts/sync-check.js— checks frontend/healthand brain concurrently usingres.okREADME.mdAdded three sections at the end: Health & Connectivity Scripts, Development Environment (VS Code/Cursor
applyToAllProfilestip), and a full Available Scripts table.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.