Merge main into 2026-03-04-lzdy: resolve dirty PR state#100
Merged
Asper Beauty Shop (asperpharma) merged 3 commits intoMar 5, 2026
Conversation
- Add .cursor/settings.json: figma and cursor-team-kit plugins - Update package.json: add health/brain/sync scripts, keep sync:dry/sync:publish for Shopify - Replace scripts/health-check.js with simplified brain+frontend version - Add scripts/brain-check.js for Beauty Assistant connectivity check - Add scripts/sync-check.js for combined sync check (SNC) - Add skills-lock.json, skills/ and .agents/skills/ directories with SKILL.md files - Update supabase/functions/beauty-assistant/index.ts: GEMINI API fallback, Deno declare, improved product context, updated system prompt - Update README.md: add SNC section, applyToAllProfiles docs, and Available scripts table Co-authored-by: asperpharma <252395498+asperpharma@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Copilot Request
Merge 2026-03-04-lzdy into latest main (brain/sync/skills infra)
Mar 5, 2026
Copilot
AI
changed the title
Merge 2026-03-04-lzdy into latest main (brain/sync/skills infra)
Merge main into 2026-03-04-lzdy: resolve dirty PR state
Mar 5, 2026
…52-4605-af52-88efa76ba751 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 |
Asper Beauty Shop (asperpharma)
deleted the
copilot/fix-252395498-1157785581-0a3862fd-1c52-4605-af52-88efa76ba751
branch
Copilot started reviewing on behalf of
Asper Beauty Shop (asperpharma)
March 5, 2026 13:24
View session
There was a problem hiding this comment.
Pull request overview
Resolves the “dirty” mergeable state by merging main into the feature branch and choosing a single implementation for the newly-added scripts/health-check.js, while also updating the Beauty Assistant edge function and adding Cursor/skills configuration to support new workflows.
Changes:
- Resolved the add/add conflict for
scripts/health-check.jsby keeping the fetch-based implementation. - Updated
supabase/functions/beauty-assistant/index.ts(CORS behavior, webhook handling, AI key fallback, and SSE streaming path). - Added new skill docs/config (
skills/*,.agents/skills/*,.cursor/settings.json,skills-lock.json) and updated npm scripts.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/functions/beauty-assistant/index.ts | Updates CORS behavior, webhook handling, and Gemini/Lovable streaming logic. |
| scripts/health-check.js | Simplifies health checks to fetch-based probes of frontend + edge function. |
| package.json | Updates npm scripts for health/brain/sync checks (but currently introduces script issues). |
| package-lock.json | Lockfile update including new transitive type package entry. |
| skills/get-pr-comments/SKILL.md | Adds documentation for fetching PR comments via gh CLI. |
| skills/code-connect-components/SKILL.md | Adds skill doc for wiring components / Figma Code Connect workflow. |
| .agents/skills/code-connect-components/SKILL.md | Adds detailed agent skill for Figma Code Connect mappings. |
| skills-lock.json | Adds skills lock metadata for installed skill(s). |
| .cursor/settings.json | Enables additional Cursor plugins (figma, cursor-team-kit). |
Comments suppressed due to low confidence (2)
package.json:27
scriptscontains duplicate keys (health/brain) which is invalid JSON (only the last value will win). Remove the duplicate entries so each script name is defined once.
"health": "node scripts/health-check.js",
"brain": "node scripts/brain-check.js",
"sync:check": "node scripts/sync-check.js"
supabase/functions/beauty-assistant/index.ts:97
formatProductwas changed to acceptany, which removes type safety around the fields used (title/brand/price/etc.) and makes accidental undefined access easier. Consider keepingRecord<string, unknown>(or introducing a minimalProductRowtype) and narrowing/validating the properties you read.
/** Format a product row into a readable string for the AI context */
function formatProduct(p: any): string {
const parts = [`**${p.title}**`];
if (p.brand) parts[0] += ` (${p.brand})`;
if (p.price) parts.push(`${p.price} JOD`);
if (p.regimen_step) parts.push(p.regimen_step.replace(/^Step_\d+_?/, "").replace(/([A-Z])/g, " $1").trim());
Comment on lines
+20
to
+22
| "health": "node scripts/health-check.js", | ||
| "brain": "node scripts/brain-check.js", | ||
| "sync": "node scripts/sync-check.js", |
There was a problem hiding this comment.
The sync script now points to scripts/sync-check.js, but docs in this repo treat npm run sync as the catalog sync command (with sync:dry/sync:publish variants). Consider restoring sync to the Shopify catalog sync script and keeping connectivity checks under sync:check to avoid breaking the documented workflow.
Suggested change
| "health": "node scripts/health-check.js", | |
| "brain": "node scripts/brain-check.js", | |
| "sync": "node scripts/sync-check.js", | |
| "sync": "tsx scripts/sync-shopify-catalog.ts", |
Comment on lines
+7
to
+16
| 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 TIMEOUT_MS = 10000; | ||
|
|
||
| /** | ||
| * Makes an HTTP GET request and returns response data | ||
| */ | ||
| function fetchUrl(url) { | ||
| return new Promise((resolve, reject) => { | ||
| const parsedUrl = new URL(url); | ||
| const client = parsedUrl.protocol === 'https:' ? https : http; | ||
|
|
||
| const options = { | ||
| hostname: parsedUrl.hostname, | ||
| port: parsedUrl.port, | ||
| path: parsedUrl.pathname + parsedUrl.search, | ||
| method: 'GET', | ||
| timeout: TIMEOUT_MS, | ||
| headers: { | ||
| 'User-Agent': 'Asper-Health-Check/1.0' | ||
| } | ||
| }; | ||
|
|
||
| const req = client.request(options, (res) => { | ||
| let data = ''; | ||
| res.on('data', (chunk) => { data += chunk; }); | ||
| res.on('end', () => { | ||
| resolve({ | ||
| statusCode: res.statusCode, | ||
| headers: res.headers, | ||
| body: data | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| req.on('error', reject); | ||
| req.on('timeout', () => { | ||
| req.destroy(); | ||
| reject(new Error('Request timeout')); | ||
| }); | ||
|
|
||
| req.end(); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Check a single endpoint | ||
| */ | ||
| async function checkEndpoint(name, url, expectedStatus = 200) { | ||
| try { | ||
| const response = await fetchUrl(url); | ||
| const success = response.statusCode === expectedStatus; | ||
|
|
||
| console.log(`[${success ? '✓' : '✗'}] ${name}`); | ||
| console.log(` URL: ${url}`); | ||
| console.log(` Status: ${response.statusCode} ${success ? '(OK)' : `(Expected ${expectedStatus})`}`); | ||
|
|
||
| if (!success) { | ||
| console.log(` Body preview: ${response.body.substring(0, 200)}`); | ||
| } | ||
|
|
||
| return { name, url, success, statusCode: response.statusCode }; | ||
| } catch (error) { | ||
| console.log(`[✗] ${name}`); | ||
| console.log(` URL: ${url}`); | ||
| console.log(` Error: ${error.message}`); | ||
| return { name, url, success: false, error: error.message }; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check health endpoint and parse JSON response | ||
| */ | ||
| async function checkHealthEndpoint() { | ||
| const url = `${SITE_URL}/health`; | ||
| console.log('\n🏥 Checking Health Endpoint...'); | ||
|
|
||
| async function check(name, url) { | ||
| try { | ||
| const response = await fetchUrl(url); | ||
|
|
||
| if (response.statusCode !== 200) { | ||
| console.log(`[✗] Health endpoint`); | ||
| console.log(` Status: ${response.statusCode}`); | ||
| return { success: false, statusCode: response.statusCode }; | ||
| } | ||
|
|
||
| let healthData; | ||
| try { | ||
| healthData = JSON.parse(response.body); | ||
| } catch (e) { | ||
| console.log(`[✗] Health endpoint - Invalid JSON`); | ||
| return { success: false, error: 'Invalid JSON response' }; | ||
| } | ||
|
|
||
| console.log(`[✓] Health endpoint`); | ||
| console.log(` Status: ${healthData.status}`); | ||
| console.log(` Version: ${healthData.version}`); | ||
| console.log(` Supabase: ${healthData.checks?.supabase ? '✓' : '✗'}`); | ||
| console.log(` Shopify: ${healthData.checks?.shopify ? '✓' : '✗'}`); | ||
|
|
||
| return { | ||
| success: healthData.status === 'ok', | ||
| data: healthData | ||
| }; | ||
| } catch (error) { | ||
| console.log(`[✗] Health endpoint`); | ||
| console.log(` Error: ${error.message}`); | ||
| return { success: false, error: error.message }; | ||
| const res = await fetch(url, { method: "GET" }); | ||
| return { name, ok: res.ok, status: res.status, url }; | ||
| } catch (err) { | ||
| return { name, ok: false, status: null, error: err.message, url }; |
There was a problem hiding this comment.
This health check no longer supports the SITE_URL override documented in the README, and it also removed the previous request timeout. Consider reintroducing an env override (e.g., SITE_URL/FRONTEND_HEALTH/BRAIN_URL) and using an AbortController timeout so the script can’t hang indefinitely on network issues.
|
|
||
| const allowOrigin: string = | ||
| Deno.env.get("ALLOWED_ORIGIN") ?? | ||
| req.headers.get("Origin") ?? |
There was a problem hiding this comment.
getCorsHeaders falls back to reflecting the request’s Origin when ALLOWED_ORIGIN is unset, which effectively enables CORS for any origin (and includes authorization in allowed headers). If this endpoint is intended to be called only from known frontends, consider defaulting to a fixed allowlist (or * consistently) rather than echoing arbitrary origins.
Suggested change
| req.headers.get("Origin") ?? |
Comment on lines
+214
to
219
| // ——— Webhook Path (Gorgias / ManyChat) — no auth ——— | ||
| if (route === "gorgias" || route === "manychat") { | ||
| const webhookStartMs = Date.now(); | ||
| // Admin client for audit writes (service role bypasses RLS) | ||
| const auditClient = createClient( | ||
| Deno.env.get("SUPABASE_URL")!, | ||
| Deno.env.get("SUPABASE_SERVICE_ROLE_KEY") ?? Deno.env.get("SUPABASE_ANON_KEY")!, | ||
| ); | ||
|
|
||
| async function logWebhookAudit(status: string, concernDetected: string | null, errorMessage: string | null) { | ||
| try { | ||
| await auditClient.from("webhook_audit_logs").insert({ | ||
| provider: route, | ||
| event_type: "message", | ||
| status, | ||
| concern_detected: concernDetected, | ||
| response_ms: Date.now() - webhookStartMs, | ||
| error_message: errorMessage ? errorMessage.slice(0, 500) : null, | ||
| }); | ||
| } catch (e) { | ||
| console.error("Audit log insert failed:", e); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| // Rate limit BEFORE crypto work to block floods cheaply | ||
| const clientIp = req.headers.get("x-forwarded-for")?.split(",")[0]?.trim() | ||
| ?? req.headers.get("x-real-ip") ?? "unknown"; | ||
| if (!webhookRateLimit(clientIp, route)) { | ||
| console.warn(`Webhook rate limit exceeded: ${route} from ${clientIp}`); | ||
| await logWebhookAudit("rate_limited", null, `IP: ${clientIp}`); | ||
| return new Response(JSON.stringify({ error: "Too many requests" }), { | ||
| status: 429, headers: { ...getCorsHeaders(req), "Content-Type": "application/json", "Retry-After": "60" }, | ||
| }); | ||
| } | ||
|
|
||
| const rawBody = await req.text(); | ||
|
|
||
| // Verify webhook signature | ||
| const webhookSecret = route === "gorgias" | ||
| ? Deno.env.get("GORGIAS_WEBHOOK_SECRET") | ||
| : Deno.env.get("MANYCHAT_WEBHOOK_SECRET"); | ||
| const signature = route === "gorgias" | ||
| ? req.headers.get("x-gorgias-signature") | ||
| : req.headers.get("x-hub-signature-256") ?? req.headers.get("x-hub-signature"); | ||
|
|
||
| if (!webhookSecret) { | ||
| console.error(`${route} webhook secret not configured`); | ||
| await logWebhookAudit("error", null, "Webhook secret not configured"); | ||
| return new Response(JSON.stringify({ error: "Webhook not configured" }), { | ||
| status: 503, headers: { ...getCorsHeaders(req), "Content-Type": "application/json" }, | ||
| }); | ||
| } | ||
|
|
||
| const valid = await verifyWebhookSignature(rawBody, signature, webhookSecret); | ||
| if (!valid) { | ||
| console.warn(`Invalid ${route} webhook signature`); | ||
| await logWebhookAudit("hmac_failed", null, "Invalid HMAC signature"); | ||
| return new Response(JSON.stringify({ error: "Invalid signature" }), { | ||
| status: 401, headers: { ...getCorsHeaders(req), "Content-Type": "application/json" }, | ||
| }); | ||
| } | ||
|
|
||
| let body: Record<string, unknown> = {}; | ||
| try { body = JSON.parse(rawBody); } catch { body = {}; } | ||
| try { body = await req.json(); } catch { body = {}; } | ||
| const { message: userMessage } = route === "gorgias" ? extractFromGorgias(body) : extractFromManyChat(body); |
There was a problem hiding this comment.
The webhook handler is explicitly marked “no auth” and the earlier HMAC signature verification + rate limiting/audit logging were removed. This makes the edge function callable by anyone to trigger AI requests and product lookups (cost/DoS risk, and potentially unintended data exposure). Please restore signature verification (provider-specific secrets) and basic rate limiting, or add an alternate authentication mechanism for webhook routes.
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
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.
PR #88 was blocked (
mergeable_state: dirty) due to an add/add conflict inscripts/health-check.js— bothmainand the feature branch independently introduced this file with different implementations.Changes
Conflict resolution
origin/main(550bf18) into2026-03-04-lzdywith--allow-unrelated-histories -X oursscripts/health-check.jsadd/add conflict — kept the PR branch's fetch-based version (checks both frontend/healthand Beauty Assistant connectivity) over main'shttps/httpmodule versionMerge commit state
d9b4948on local2026-03-04-lzdywith parents1164a38(PR head, includes 3 code-review suggestion commits) +550bf18(main with dependabot + PR [WIP] Add documentation for sync command and available scripts #103)brain-check.js,health-check.js,sync-check.js, updatedbeauty-assistant/index.ts(Gemini API fallback + SSE streaming fixes),package.jsonhealth/brain/syncscriptsBranch protection on
mainblocks the automated push path (report_progressonly writes toorigin/main). To finalize:Alternatively, use GitHub's Update branch button on the PR page.
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.