feat: Grida AI Credit (v1) + AI seam consolidation#711
Merged
Conversation
- Add SECURITY.md as the registry for prevented-vulnerability ids - Allocate GRIDA-SEC-001 for the ingest trust boundary (webhook receivers exposed on a static URL must HMAC-verify before any business logic) - Add (ingest) route group with README codifying the rules - Move Stripe webhook receiver from (api)/private/webhooks/stripe to (ingest)/webhooks/stripe; URL changes to /webhooks/stripe - Add /webhooks/* bypass in proxy.ts (skips tenant routing + session refresh) - Add .agents/skills/security/SKILL.md — auto-loads on any GRIDA-SEC mention, mandates security review before commit on tagged files - Update E2E fixtures and .env.example to the new path
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds Metronome-backed AI credits, an AI usage seam with org gating and metered ingestion, signed webhook receivers, DB migrations/RPCs, UI/Checkout flows, CLI tooling, lint/audit enforcement, and tests. ChangesAI Credits and AI Seam Integration Schema & DB RPCs
Server seam & billing service
Ingress, proxy, cron
Server actions & client credits
Product surfaces
Security, docs, lint, audit
Tooling & tests
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Org Billing UI
participant Stripe
participant Ingest as /webhooks/stripe
participant DB as Supabase RPC
participant Billing as lib/billing/metronome.ts
User->>UI: Start top-up (Checkout)
UI->>Stripe: Create Checkout Session
Stripe->>Ingest: webhook checkout.session.completed (raw body)
Ingest->>DB: fn_billing_get_ai_credit_processed(event_id)
DB-->>Ingest: processed?
Ingest->>Billing: handleAiCreditCheckoutCompleted(session)
Billing->>DB: fn_billing_set_balance_cache / addStripeChargedCommit
Billing-->>Ingest: applied/noop
UI->>Billing: refreshAiCreditsBalance()
Billing-->>UI: { cents, allowed }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Three subcommands under one entry point: `/database compact local migration` for safely consolidating only unshipped migrations, `/database rls scenarios` for writing pgTAP coverage where the implementation mirrors the test (not the reverse), and `/database align` for keeping `schemas/*.sql` in sync with the migrated state. Encodes the discipline that applied migrations are immutable history — classification heuristics are signals, not authority; user confirmation is the only ground truth.
Schema: per-org Metronome customer/contract ids + entitlement-cache + auto-reload columns on grida_billing.account, plus metronome_event dedup table and the projector RPC. Multi-tier-alert aware (only depletion-tier flips entitlement off; warning tiers refresh balance only). Service: editor/lib/billing/metronome.ts — provisionOrg, addStripeChargedCommit, addComplimentaryCommit, setAutoReload, ingestUsageEvent[Gated], getEntitlement, getOrgBalance, getTransactions, getInvoices, refreshBalance, revokeUnusedOnCommit, getAccountView, handleAiCreditCheckoutCompleted (post-Checkout reconciler). Money model: fees.ts ships a flat 5% + \$0.30 gross-up markup envelope verified safe across every Stripe card type at \$10-\$500 (audit via cli.ts markup-sim). AI_GATE_FLOOR_CENTS is the single source for the 25c gate floor.
Metronome receiver: HMAC-SHA256 over Date+raw-body, 5-min freshness, dedup via the projector's PK on event_id, payment_gate failure logged with reason, post-projector refreshBalance for commit-affecting events. Stripe receiver: extends the existing projector with two ai-credit branches — checkout.session.completed (kind=ai_topup or ai_auto_reload_enable) routes to handleAiCreditCheckoutCompleted; customer.subscription.deleted disables auto-reload (KI-BILL-001 mitigation: never silent-recharge a canceled org). Errors return 500 so Stripe retries instead of leaving the customer charged with no credit. Reconcile cron: hourly /internal/cron/billing-reconcile sweeps only provisioned orgs (via fn_billing_list_provisioned_orgs) with concurrency 4.
Settings → Billing now hosts a Grida AI Credit panel: balance card with two-step Buy Credit dialog ($X-of-credit picker → line-item breakdown showing markup as "Payment Processing Fee"), collapsible auto-reload configuration (subscription-gated per KI-BILL-001), and a recent-activity feed. Server actions in _actions.ts: getAiCreditsSummary (lazy-provisions only when not yet wired — skips on every steady-state poll), Stripe Checkout launchers for top-up and auto-reload-enable with payment_intent_data + invoice_creation so post-payment Stripe Invoices appear in Past Invoices. Edit-in-place auto-reload mutators don't re-Checkout. Return page (post-Stripe-Checkout interstitial) settles on `entitled === true` rather than a delta vs baseline — kills a race where the webhook landed before the page mounted and polling never settled. Polling pauses when the tab is hidden and stops entirely once state is steady, so an open billing tab doesn't hammer Stripe + Metronome.
The insiders harness drives every Metronome/Stripe lifecycle step manually (provision, add commit, ingest, alerts, refund, invoices, ...) — useful for debugging the live system without writing scripts. The harness's server actions intentionally skip org-membership checks, so the route group MUST never be reachable outside local development. GRIDA-SEC-002 enforces that contract at two layers: - editor/proxy.ts — 404 every /insiders/* request when NODE_ENV != "development", including Next-Action POSTs. - editor/app/(insiders)/layout.tsx — defense-in-depth notFound() in the layout if any request slips past the proxy. SECURITY.md documents both layers and lists every file bound by the id.
27 ad-hoc scripts → 6 files: cli.ts single entry point with subcommands _env.ts shared env loader (.env.test.local > .env.test > .env.local) setup.ts setup:stripe, setup:metronome smoke.ts ping, smoke:topup, smoke:auto-reload, smoke:webhook ops.ts backfill, markup-sim README.md Drops: - All `_*` underscore-prefix diagnostics (transient debug artifacts). - All mid-development spike scripts (overage-test, spike-experiments, stripe-poc, refire-test, empty-window, drift-test, multi-tier-alerts, optimistic-debit) — proven and folded into code/docs. - tunnel.sh — locally-configured cloudflared per docs/contributing/billing.md.
Drives `handleAiCreditCheckoutCompleted` directly with a synthetic Stripe Checkout session — no browser, no real Checkout UI, no dev server. The test asserts: - the handler returns `applied` - DB customer_entitled flips to true synchronously (proves the inline reconcile closes Gap B for the happy path) - DB cached_balance_cents reflects the new credit - the transaction's `at` field is wall-clock-fresh (Commit.created_at, not the hour-floored schedule_items[0].starting_at) vitest.config.ts now also falls back to .env.local for env vars, mirroring what the cli.ts scripts already do — a token kept there for the dev server no longer has to be duplicated into .env.test.local.
Three working-group docs under docs/wg/platform/: - ai-credits.md the master design (constraints, flows, drain order, refunds) - metronome.md the Metronome integration playbook - billing-known-issues.md living register; KI-BILL-001 (silent auto-recharge markup gap, mitigated by subscription gate) + KI-BILL-002 (concurrent subscribe-Checkout race, accepted for v1). Contributor setup (docs/contributing/billing.md) is rewritten for the consolidated CLI (`cli.ts setup:stripe`, `setup:metronome`, etc.) and the locally-configured cloudflared tunnel (no longer ships a wrapper script). editor/.env.example documents METRONOME_API_TOKEN, METRONOME_WEBHOOK_SECRET, and WEBHOOK_TUNNEL_HOSTNAME alongside the existing Stripe vars.
Three flat docs under wg/platform/ are now grouped: wg/platform/ai-credits.md → wg/platform/billing/ai-credits.md wg/platform/billing-known-issues.md → wg/platform/billing/known-issues.md wg/platform/metronome.md → wg/platform/billing/metronome.md Plus a billing/index.md and _category_.json so Docusaurus renders the group as a labeled section. wg/platform/index.md links to the new section. Inside known-issues.md, the redundant `billing-` prefix is dropped since the directory now provides the namespace. All external references updated to the new paths: docs/contributing/billing.md, service module + fees comment headers, the consolidated migration, _actions.ts, the Stripe webhook receiver, and the scripts CLI README. Verified zero stale references via grep across docs/, editor/, supabase/.
The formatter parsed "(Stripe + Metronome ..." as a sub-bullet because of the leading hyphen. Reword to a single-bullet phrase.
softmarshmallow
changed the title
softmarshmallow
added
migration
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4134eb83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+92
to
+94
| if (result.result === "replayed") { | ||
| return NextResponse.json({ received: true, replayed: true }); | ||
| } |
There was a problem hiding this comment.
dispatchStripeEvent records/deduplicates the Stripe event before handleAiCreditCheckoutCompleted runs, but this early return exits on result === "replayed" before the AI-credit branch executes. If the first delivery fails in the post-processor (returns 500), Stripe retries with the same event id; on retry this branch returns early and the credit/top-up side effect is skipped forever, leaving a paid checkout without credited balance. This affects checkout.session.completed retries specifically.
Useful? React with 👍 / 👎.
| typeof g.costMills === "number" && g.costMills >= 0 | ||
| ? g.costMills | ||
| : undefined, | ||
| awaitIngest: g.awaitIngest === true, |
There was a problem hiding this comment.
extractContext currently forces awaitIngest to false unless callers explicitly pass true (g.awaitIngest === true), which overrides the intended default in withTransaction (ctx.awaitIngest ?? true). As a result, normal non-streaming AI calls become fire-and-forget ingest by default, so withAiAuth can return a stale post-call balance before usage debit is ingested/reconciled.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
editor/scaffolds/playground-forms/playground.tsx (1)
109-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
.catchwill leave the UI stuck in a "Generating…" state.With
organizationIdnow passed asundefined,generate(...)is expected to throw a400server-side. The current.then(...)chain has no.catch/.finally, so on rejectiongenerating.currentstaystrueandbusystaystrueforever — the spinner never clears and the user can't retry. Even if the public playground is intended to be temporarily unusable until the sign-in gate lands, the failure mode should be a recoverable error toast rather than a frozen header.🛠️ Proposed fix
- generate(undefined, prompt, initial?.slug).then(async ({ output }) => { - for await (const delta of readStreamableValue(output)) { - // setData(delta as JSONForm); - __set_schema_txt(JSON.stringify(delta, null, 2)); - } - generating.current = false; - setBusy(false); - }); + generate(undefined, prompt, initial?.slug) + .then(async ({ output }) => { + for await (const delta of readStreamableValue(output)) { + __set_schema_txt(JSON.stringify(delta, null, 2)); + } + }) + .catch((err) => { + console.error(err); + toast.error("Generation failed. Please sign in to use this feature."); + }) + .finally(() => { + generating.current = false; + setBusy(false); + });🤖 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 `@editor/scaffolds/playground-forms/playground.tsx` around lines 109 - 128, streamGeneration currently calls generate(...).then(...) but has no rejection handler, so on error (e.g., generate(undefined,...)) generating.current and busy never reset; add error handling to the promise: attach a .catch that sets generating.current = false and setBusy(false) and surfaces an error toast, and a .finally that ensures both flags are reset even if streaming started; alternatively convert to async/await with try/catch/finally around generate and the for-await loop (referencing streamGeneration, generate, readStreamableValue, generating.current, setBusy, and __set_schema_txt) so failures always clear the spinner and allow retry.editor/app/(tools)/(playground)/playground/image/_page.tsx (1)
107-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOrphaned image node on generation/credit failure.
onCommitinserts the image node before kicking off the transition. IfgenerateAiImagethrows orcredits.consumereturnsundefined(gate redirect, blocked, internal error), the earlyreturnat line 124 leaves an empty image node in the document with nosrc. The user is also redirected away (e.g.next: "/playground/image") but on return they'll find a leftover broken node.Either insert the node only after a successful result, or delete it in the failure paths (and on exception).
🔧 Proposed fix
const onCommit = (value: { text: string }) => { - const id = editor.commands.insertNode({ - type: "image", - name: value.text, - layout_target_width: model.width, - layout_target_height: model.height, - fit: "cover", - }); startGenerate(async () => { - const env = await generateAiImage({ - model: model.modelId, - width: model.width, - height: model.height, - aspect_ratio: model.aspect_ratio, - prompt: value.text, - }); - const data = credits.consume(env, { next: "/playground/image" }); - if (!data) return; // gate / redirect handled - editor.commands.changeNodePropertySrc(id, data.publicUrl); + const id = editor.commands.insertNode({ + type: "image", + name: value.text, + layout_target_width: model.width, + layout_target_height: model.height, + fit: "cover", + }); + try { + const env = await generateAiImage({ + model: model.modelId, + width: model.width, + height: model.height, + aspect_ratio: model.aspect_ratio, + prompt: value.text, + }); + const data = credits.consume(env, { next: "/playground/image" }); + if (!data) { + editor.commands.deleteNode(id); // adjust to actual API + return; + } + editor.commands.changeNodePropertySrc(id, data.publicUrl); + } catch (e) { + editor.commands.deleteNode(id); + throw e; + } }); };🤖 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 `@editor/app/`(tools)/(playground)/playground/image/_page.tsx around lines 107 - 127, The onCommit handler currently calls editor.commands.insertNode (creating an image node) before invoking startGenerate/generateAiImage and credits.consume, which leaves an empty image node if generation or credit consumption fails; update onCommit so the image node is only inserted after a successful generateAiImage + credits.consume result (or, if you prefer minimal change, capture the returned node id from editor.commands.insertNode and ensure you remove that node in all failure paths and catch blocks), and then call editor.commands.changeNodePropertySrc(id, data.publicUrl) only after a successful data is obtained; reference functions: onCommit, startGenerate, generateAiImage, credits.consume, editor.commands.insertNode, editor.commands.changeNodePropertySrc.editor/grida-canvas-hosted/ai/tools/canvas-use.ts (1)
126-146:⚠️ Potential issue | 🔴 CriticalAdd runtime validation for aspect_ratio format before type assertion.
The
aspect_ratioparameter is cast to${number}:${number}(line 138) using a TypeScript type assertion without runtime validation. TypeScript template literal types are compile-time constructs only; invalid formats like"16x9"or"wide"will pass the assertion. Since the aspect_ratio comes directly from user input, validate it against a list of allowed formats (similar to the validation pattern inuse-image-model.tslines 68–79) before callinggenerateAiImage.🤖 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 `@editor/grida-canvas-hosted/ai/tools/canvas-use.ts` around lines 126 - 146, The execute function is unsafely asserting aspect_ratio to `${number}:${number}` before runtime validation; before calling generateAiImage validate the incoming aspect_ratio against the allowed set/pattern (same approach used in use-image-model.ts) and only pass a validated value (falling back to "1:1" if invalid) to generateAiImage; update the validation logic inside execute (referencing the execute method and generateAiImage call) to check the format (e.g., regex or allowed list) and reject or normalize bad inputs prior to the type assertion/call.editor/app/(api)/private/ai/chat/route.ts (1)
98-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn the same AI error envelope on 500s.
Auth and organization failures now use
aiErrorResponse, but the catch path falls back to plain text. That reintroduces a second error contract for the same endpoint and makes client-side error parsing inconsistent.Consistent 500 response
} catch (error) { console.error("Error in agent chat:", error); - return new Response("Internal error", { status: 500 }); + return aiErrorResponse({ + code: "internal_error", + status: 500, + message: "internal error", + }); } }🤖 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 `@editor/app/`(api)/private/ai/chat/route.ts around lines 98 - 100, The catch block in the agent chat route currently returns a plain text Response; change it to return the standardized AI error envelope using the existing aiErrorResponse helper so 500 responses match auth/org failures. In the catch for the route handler in editor/app/(api)/private/ai/chat/route.ts, replace the plain Response("Internal error", { status: 500 }) with a call to aiErrorResponse(error, 500) (or aiErrorResponse(new Error("Internal error"), 500) if the helper expects an Error) and ensure the caught error is passed into aiErrorResponse so the endpoint always returns the same error contract.
🟠 Major comments (20)
editor/.env.example-82-84 (1)
82-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid committing concrete secret-like test keys in the env template.
Even for test mode, shipping an actual
*_SECRET_KEY-looking value increases leakage/scanner risk. Use placeholders in.env.exampleand keep real values out of VCS.Suggested fix
-INTEGRATIONS_TEST_TOSSPAYMENTS_CUSTOMER_KEY="test_ck_ORzdMaqN3w92ng0ALboPr5AkYXQG" -INTEGRATIONS_TEST_TOSSPAYMENTS_SECRET_KEY="test_sk_6BYq7GWPVvyJpbAv24nw3NE5vbo1" +INTEGRATIONS_TEST_TOSSPAYMENTS_CUSTOMER_KEY="test_ck_..." +INTEGRATIONS_TEST_TOSSPAYMENTS_SECRET_KEY="test_sk_..."🤖 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 `@editor/.env.example` around lines 82 - 84, Replace the concrete test keys in .env.example with non-secret placeholders: remove the literal values assigned to INTEGRATIONS_TEST_TOSSPAYMENTS_CUSTOMER_KEY and INTEGRATIONS_TEST_TOSSPAYMENTS_SECRET_KEY and replace them with descriptive placeholders (e.g., "YOUR_TOSSPAYMENTS_CUSTOMER_KEY" and "YOUR_TOSSPAYMENTS_SECRET_KEY") so no secret-looking tokens are committed; ensure the variable names remain unchanged so consumers know what to set.editor/lib/billing/metronome.ts-1522-1588 (1)
1522-1588:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't mint credit from Checkout metadata without re-validating the payment.
Both branches grant
addComplimentaryCommit()based onmetadataalone. If a session is created with a mismatchedamount_totalor out-of-range*_cents, the webhook will still issue credit that Stripe never collected; in the auto-reload branch that happens beforesetAutoReload()can reject the config. Recompute the expected gross withtotalChargeForCredit(...), enforce the same bounds/whole-dollar rules here, and only add the commit after those checks pass.🤖 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 `@editor/lib/billing/metronome.ts` around lines 1522 - 1588, The code mints credit from session.metadata without re-validating the actual charged amount; before calling addComplimentaryCommit (in both TOPUP and AUTO_RELOAD_ENABLE branches) recompute the expected gross using totalChargeForCredit(...) with the same inputs (cents/threshold/recharge and any currency/fee params), verify it matches session.amount_total and that the cents/threshold/recharge values pass the same whole-dollar and min/max bounds enforced elsewhere, and only proceed to call addComplimentaryCommit/refreshBalance/setAutoReload after those checks pass; update the validation logic around parseInt(meta.cents), parseInt(meta.threshold_cents), and parseInt(meta.recharge_to_cents) to use the totalChargeForCredit result and existing AUTO_RELOAD_*/TOPUP bounds before granting credit.supabase/migrations/20260508130000_grida_billing_metronome.sql-84-86 (1)
84-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRevoke
PUBLICbefore grantingservice_role.This table stores raw billing webhook payloads. Only revoking
anonandauthenticatedstill leaves privilege safety dependent on default grants. Add an explicitREVOKE ALL ON TABLE grida_billing.metronome_event FROM PUBLIC;before the grant.Proposed fix
ALTER TABLE grida_billing.metronome_event ENABLE ROW LEVEL SECURITY; +REVOKE ALL ON TABLE grida_billing.metronome_event FROM PUBLIC; REVOKE ALL ON TABLE grida_billing.metronome_event FROM anon, authenticated; GRANT ALL ON TABLE grida_billing.metronome_event TO service_role;As per coding guidelines,
Explicitly grant permissions only to required roles; avoid accidental PUBLIC access by using \REVOKE ALL FROM PUBLIC;` followed by specific role grants`.🤖 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/migrations/20260508130000_grida_billing_metronome.sql` around lines 84 - 86, Add an explicit revoke for PUBLIC on the table grida_billing.metronome_event before granting to service_role: call REVOKE ALL ON TABLE grida_billing.metronome_event FROM PUBLIC prior to the GRANT, and ensure the sequence is: ENABLE ROW LEVEL SECURITY, REVOKE ALL FROM PUBLIC, REVOKE ALL FROM anon, authenticated, then GRANT ALL TO service_role so the table is not accidentally accessible via default PUBLIC privileges.editor/lib/billing/metronome.ts-1035-1044 (1)
1035-1044:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompare balance drift at the same precision as the cache.
reconcileAccountFromLive()floors the live balance into integer cents before persisting it, but this comparison uses the raw fractionallive.balanceCents. After any mills-based debit,drift.balanceCentswill staytrueeven when the cache is perfectly synchronized.Proposed fix
- drift.balanceCents = db.cached_balance_cents !== live.balanceCents; + drift.balanceCents = + db.cached_balance_cents !== Math.floor(live.balanceCents);🤖 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 `@editor/lib/billing/metronome.ts` around lines 1035 - 1044, drift.balanceCents is being set by comparing db.cached_balance_cents to the raw fractional live.balanceCents, but reconcileAccountFromLive() floors the live balance to integer cents before persisting; update the comparison in reconcileAccountFromLive() (or where drift.balanceCents is computed) to compare db.cached_balance_cents against the floored live value (e.g., Math.floor(live.balanceCents)) so both sides use the same integer-cent precision (handle null/undefined the same way as the other balance checks).editor/.oxlintrc.jsonc-24-39 (1)
24-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict direct provider package subpaths to close security seam
Lines 24–39 block bare package imports (
openai,replicate,@anthropic-ai/sdk), but the"name"field does not restrict subpath imports. Oxlint'sno-restricted-importsrequires"patterns"with wildcards to block subpaths likeopenai/resources/*. Add pattern restrictions to prevent GRIDA-SEC-003 bypass.Suggested config hardening
"patterns": [ + { + "group": ["openai/*", "replicate/*", "@anthropic-ai/sdk/*"], + "message": "Use editor/lib/ai/server instead. GRIDA-SEC-003.", + "allowTypeImports": true + }, { // Provider-specific AI SDK packages. Excludes `@ai-sdk/rsc` // (server-component streaming utilities) and `@ai-sdk/react`🤖 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 `@editor/.oxlintrc.jsonc` around lines 24 - 39, The current no-restricted-imports entries for "replicate", "openai", and "@anthropic-ai/sdk" only block the bare package name and do not stop subpath imports; update each restricted-import object (the entries whose "name" values are replicate, openai, and `@anthropic-ai/sdk`) to include a "patterns" array with wildcard entries (e.g., "<package>/*") that mirror the "name" block so any subpath like openai/resources/* or `@anthropic-ai/sdk/dist/`* is rejected, keep "allowTypeImports": true and reuse the same "message" text for consistency.editor/grida-canvas-react-starter-kit/starterkit-toolbar/image-toolbar.tsx-309-321 (1)
309-321:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTODO leaves Upscale/Remove-Background non-functional until host threads
organizationId.Per the AI summary and the inline TODO at lines 309-312, the server action returns
400 (GRIDA-SEC-003)without anorganizationId. Since this toolbar doesn't yet thread one through, every click on Upscale (and Remove Background at lines 374-380) will fail at the seam and surface a generic toast/redirect viahandleApiError. Consider one of:
- Plumb an
organizationId(or resolver) prop intoImageToolbarnow and forward it into the action calls.- Disable/hide the buttons when no org context is available, so users don't hit guaranteed-400 paths.
Same applies to the
removeBackgroundImagecall at line 376.Want me to draft a
organizationId?: numberprop onImageToolbarand wire it through both action calls?🤖 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 `@editor/grida-canvas-react-starter-kit/starterkit-toolbar/image-toolbar.tsx` around lines 309 - 321, The toolbar currently calls upscaleImage and removeBackgroundImage without an organization context so the server returns GRIDA-SEC-003; add an optional prop to ImageToolbar (e.g., organizationId?: number or organizationResolver?: ()=>number|Promise<number>), accept it in the ImageToolbar props and thread it into both upscaleImage({ image, scale, organizationId }) and removeBackgroundImage({ image, organizationId }), and as a fallback disable or hide the Upscale/Remove Background buttons when no organizationId/resolver is provided so users cannot trigger guaranteed-400 server actions.editor/app/(api)/internal/cron/billing-reconcile/route.ts-27-34 (1)
27-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop
?secret=authentication for the cron endpoint.Putting
CRON_SECRETin the URL leaks it into browser history and request logs. Since this route already supportsAuthorization: Bearer, keeping auth header-only closes that exposure without affecting the scheduled caller.Safer auth check
function authorized(req: NextRequest): boolean { const secret = process.env.CRON_SECRET; if (!secret) return false; const header = req.headers.get("authorization"); - if (header && header === `Bearer ${secret}`) return true; - const url = new URL(req.url); - if (url.searchParams.get("secret") === secret) return true; - return false; + return header === `Bearer ${secret}`; }🤖 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 `@editor/app/`(api)/internal/cron/billing-reconcile/route.ts around lines 27 - 34, The authorized function currently accepts a secret via URL query which leaks secrets; change authorized(req: NextRequest) so it only validates process.env.CRON_SECRET via the Authorization header (check req.headers.get("authorization") === `Bearer ${secret}`) and remove the URL/searchParams check that reads req.url and compares "secret"; keep the early false when secret is unset and ensure any callers/scheduled callers use the Authorization: Bearer header exclusively.editor/app/(site)/organizations/[organization_name]/settings/billing/return/_view.tsx-63-72 (1)
63-72:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
topupsettles too early for orgs that were already entitled.For a user buying additional credit before hitting zero,
a.entitled === trueis already true on the first poll, so this page can redirect before the new balance or activity lands. That makes the return flow look complete while the paid top-up is still invisible.This needs a settlement signal tied to the checkout flow itself — e.g. baseline
cents, a new activity row, or the checkout session id — not entitlement alone.🤖 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 `@editor/app/`(site)/organizations/[organization_name]/settings/billing/return/_view.tsx around lines 63 - 72, The current topup branch (case "topup") returns true based solely on snap.ai.entitled which can be already true and causes premature settlement; modify the logic in the "topup" case (where a = snap.ai) to detect a change tied to the checkout flow instead of entitlement alone — for example, compare a.cents against a baselineCents stored before checkout, or check for a new activity entry or matching checkoutSessionId on the account activities list; update the condition to return true only when the baseline-cents delta or a new activity/checkoutSessionId indicating the just-completed purchase is observed (and keep the null check for a).editor/lib/ai/credits/actions.ts-35-43 (1)
35-43:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid forcing a live balance refresh during initial layout preload.
resolveInitialAiCredits()uses this to seed route-group layout state, sorefreshBalance()turns ordinary page renders into a Metronome-dependent path. That makes first paint slower and lets transient billing outages break otherwise cacheable pages. This preload path should read the cached balance/entitlement, with force-sync reserved forrefreshAiCredits()/webhook-reconcile flows.🤖 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 `@editor/lib/ai/credits/actions.ts` around lines 35 - 43, preloadAiCredits currently forces a live refresh via refreshBalance(orgId), which makes initial layout preload Metronome-dependent; change preloadAiCredits to read the cached balance/entitlement instead of calling refreshBalance — use the existing cached getter (e.g., getCachedBalance or readBalance) or add a small getBalanceCached(orgId) helper and call that along with getEntitlement(orgId) so resolveInitialAiCredits and route-group layout use only cached data; keep refreshBalance reserved for refreshAiCredits() and webhook-reconcile flows where a live sync is appropriate.editor/lib/ai/actions/image.ts-16-20 (1)
16-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
scalebefore forwarding it to the provider.The public contract says
scalemaxes out at 10, but this path currently forwards any runtime value. Since this is a server action, a client can still send0,NaN, or100and push that failure/cost downstream instead of getting a deterministicbad_request.Suggested fix
export async function upscaleImage( input: UpscaleImageInput ): Promise<UpscaleImageResponse> { if (!input.image) { return { success: false, code: "bad_request", message: "image is required", status: 400, }; } + if ( + input.scale !== undefined && + (!Number.isInteger(input.scale) || input.scale < 1 || input.scale > 10) + ) { + return { + success: false, + code: "bad_request", + message: "scale must be an integer between 1 and 10", + status: 400, + }; + } return withAiAuth("ai/image/upscale", input.organizationId, (orgId) => methods.upscale(orgId, { image: ai.server.methods.toImageData(input.image), scale: input.scale, })Also applies to: 26-42
🤖 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 `@editor/lib/ai/actions/image.ts` around lines 16 - 20, Validate the UpscaleImageInput.scale inside the server action that handles the upscale request (the function that receives UpscaleImageInput in this file) before forwarding to the provider: coerce/parse the value, ensure it's a finite integer between 1 and 10 (default to 4 when undefined), and throw/return a deterministic bad_request error for values like 0, NaN, negatives, or >10; do this validation in the handler that consumes UpscaleImageInput (not just in the type) and use the validated value when calling the provider so untrusted client inputs cannot be forwarded.editor/app/(ingest)/webhooks/metronome/route.ts-62-68 (1)
62-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject future-dated webhook timestamps too.
This freshness check only blocks events older than five minutes. A request dated far in the future still passes, which weakens the replay window the signature is supposed to enforce. Bound the skew in both directions here.
Suggested fix
- if (Date.now() - dateMs > FIVE_MINUTES_MS) { + if (Math.abs(Date.now() - dateMs) > FIVE_MINUTES_MS) { return NextResponse.json({ error: "stale event" }, { status: 400 }); }🤖 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 `@editor/app/`(ingest)/webhooks/metronome/route.ts around lines 62 - 68, The freshness check only rejects events older than FIVE_MINUTES_MS but allows timestamps far in the future; update the validation around dateHeader/dateMs in route.ts to enforce symmetric skew by also rejecting when dateMs - Date.now() > FIVE_MINUTES_MS (i.e., if the timestamp is more than FIVE_MINUTES_MS ahead of now). Return the same error response (NextResponse.json with status 400) for future-dated headers so the replay window is bounded in both directions.editor/lib/ai/actions/image-generate.ts-57-67 (1)
57-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded
medium/tier prefix prevents tier key lookups from ever matching.The code in
computeCostMillsconstructs tier keys as`medium/${w}x${h}`(line 60), but the pricing catalog defines all tiered models (DALLE3, DALLE3-turbo, DALLE3-mini) with keys like"low/1024x1024". This mismatch means the lookup always fails and falls through tocard.avg_cost_usd, causing high-resolution generations to be undercharged.Change
"medium/"to match the actual prefix in the pricing tier definitions, or derive the tier dynamically from request dimensions.🤖 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 `@editor/lib/ai/actions/image-generate.ts` around lines 57 - 67, The per-image tier lookup in computeCostMills (in the "per_image_tiered" case) builds tierKey as `medium/${w}x${h}` which never matches the pricing catalog keys; update the tierKey construction used in the "per_image_tiered" branch (where tierKey and perImage are computed) to use the correct prefix that matches card.pricing.tiers (e.g., replace "medium/" with the actual prefix used like "low/" or determine the tier prefix dynamically from request dimensions or card.pricing.tiers keys), so that tierKey can find an entry in card.pricing.tiers and perImage is selected from the tiered pricing instead of falling back to card.avg_cost_usd.editor/lib/ai/actions/chat.ts-99-103 (1)
99-103: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
resolvedIdand the actual model can drift.When
model_idis absent or invalid, the SDK model comes frommodel("mini")butresolvedIdis hardcoded to the string literal"openai/gpt-5.4-mini". Ifmodel("mini")is ever remapped to a different catalog id,costMillsFromTokenUsage(resolvedId, ...)will silently compute against the wrong card — i.e., the meter and the actual model disagree. DeriveresolvedIdfrom the same source the model came from so they can't diverge.♻️ Suggested fix
- const requested = input.model_id; - const useModelId = requested && isCatalogId(requested) ? requested : null; - const languageModel = useModelId ? grida(useModelId) : model("mini"); - const resolvedId = useModelId ?? catalog["openai/gpt-5.4-mini"].id; + const requested = input.model_id; + const useModelId = requested && isCatalogId(requested) ? requested : null; + const tierModel = useModelId ? null : model("mini"); + const languageModel = useModelId ? grida(useModelId) : tierModel!; + // Pull catalog id from whichever path produced the model so cost + // metering and the actual call can't drift. + const resolvedId = useModelId ?? tierModel!.modelId; // or equivalent accessorIf
model("mini")doesn't expose the catalog id, consider exporting atier("mini") → CatalogIdmapping from@/lib/ai/modelsand using that for bothgrida(...)and the cost call.🤖 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 `@editor/lib/ai/actions/chat.ts` around lines 99 - 103, The resolvedId is hardcoded to catalog["openai/gpt-5.4-mini"].id while the actual languageModel may be produced by model("mini"), which lets the two drift; change the logic in the withAiAuth block so resolvedId is derived from the same source as languageModel: if input.model_id is a catalog id use that (useModelId), otherwise derive the catalog id for the "mini" tier from the same place you call model("mini") (e.g., export/use a tier→CatalogId mapping or expose an id property from model("mini")), then pass that derived id to costMillsFromTokenUsage; update references to grida(...), model("mini"), resolvedId and any costMillsFromTokenUsage calls so they all use the same canonical catalog id source.editor/app/(site)/organizations/[organization_name]/settings/billing/_actions.ts-862-873 (1)
862-873:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror the checkout validation in
setAiAutoReload().This edit-in-place mutation skips the whole-dollar, min/max, and
threshold < recharge_tochecks thatstartEnableAutoReloadCheckout()enforces. Since it writes an off-session charge configuration directly, a malformed call can store values the UI never permits. Reuse the same validation block here beforesetAutoReload().🤖 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 `@editor/app/`(site)/organizations/[organization_name]/settings/billing/_actions.ts around lines 862 - 873, The setAiAutoReload function currently writes auto-reload settings without running the same validation used by startEnableAutoReloadCheckout; before calling setAutoReload(org_id, threshold_cents, recharge_to_cents) add the same validation logic (or call a shared validator) that enforces whole-dollar amounts, min/max bounds, and threshold_cents < recharge_to_cents as implemented in startEnableAutoReloadCheckout(), so malformed values the UI would reject cannot be stored off-session.editor/lib/ai/error.ts-63-70 (1)
63-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't echo raw exception messages back to the client.
Both mappers currently pass
err.messagethrough the AI error envelope. For unexpected org / Stripe / Metronome failures, that can leak upstream diagnostics or internal identifiers into a toast/JSON response. Log the original error, but return a fixed user-safe message for non-user-correctable failures.Also applies to: 97-110
🤖 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 `@editor/lib/ai/error.ts` around lines 63 - 70, orgErrorToAiError currently returns err.message to the client which can leak internal details; change it to log the original err (using console/error logger) but return a user-safe message instead of err.message — e.g., set message to a fixed string like "An internal error occurred" for non-user-correctable failures while preserving code via orgErrorCode(err) and status via extractStatus(err, 403); apply the same change to the other mappers in this file (the other AiErrorResponse-returning functions around the stripe/metronome mappers) so none of them echo err.message to clients.editor/lib/ai/error.ts-73-85 (1)
73-85:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't route org lookup failures into onboarding.
The default branch currently covers
org_lookup_failedand every other unknown resolver error, butresolveAiError()only looks atcode, notstatus. That means a transient 5xx during org resolution will hard-redirect users to/organizations/newinstead of surfacing an internal failure. Keep the onboarding mapping to the true “no org” cases and send lookup/backend failures to"internal"instead.🤖 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 `@editor/lib/ai/error.ts` around lines 73 - 85, The orgErrorCode function currently sends all unknown codes to "no_organization"; change its mapping so only true “no org” codes (e.g., "missing_organization_id", "org_not_found", "not_member") return "no_organization", explicitly map transient resolver failures like "org_lookup_failed" to "internal", and make the default branch return "internal" so lookup/backend errors surface as internal failures rather than redirecting to onboarding.editor/lib/ai/server.ts-318-335 (1)
318-335:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
undefinedforawaitIngestso the default still works.
extractContext()currently setsawaitIngest: g.awaitIngest === true, which turns an omitted flag intofalse.withTransaction()then never hits its?? truedefault, so plain AI SDK calls silently fall back to fire-and-forget ingest and the post-callrefreshBalance()path can observe stale credit. Only writeawaitIngestwhen the caller actually provided a boolean.Suggested fix
costMills: typeof g.costMills === "number" && g.costMills >= 0 ? g.costMills : undefined, - awaitIngest: g.awaitIngest === true, + awaitIngest: + typeof g.awaitIngest === "boolean" ? g.awaitIngest : undefined, }; }🤖 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 `@editor/lib/ai/server.ts` around lines 318 - 335, extractContext currently forces awaitIngest to true/false by using g.awaitIngest === true; change it to preserve undefined so the default in withTransaction (which uses ?? true) can apply: when building the returned object in extractContext, set awaitIngest only if the caller provided a boolean (e.g., check typeof g.awaitIngest === "boolean" and use that value; otherwise leave awaitIngest undefined) — update the object construction in function extractContext (and any related uses of g.awaitIngest) so awaitIngest is omitted/undefined unless explicitly passed.editor/lib/ai/error.ts-192-200 (1)
192-200:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
codeagainst the known union before accepting the envelope.
isAiErrorResponse()treats any string as a validcode. A malformed payload like{ success: false, code: "foo", ... }passes this guard, thenresolveAiError()falls through andhandleAiFetchErrorResponse()will blow up when it readsaction.kind. Tighten the check to the actualAiErrorCodeset.Suggested fix
+const AI_ERROR_CODES = new Set<AiErrorCode>([ + "unauthorized", + "no_organization", + "blocked", + "bad_request", + "internal", +]); + export function isAiErrorResponse(x: unknown): x is AiErrorResponse { if (!x || typeof x !== "object") return false; const o = x as Record<string, unknown>; return ( o.success === false && - typeof o.code === "string" && + typeof o.code === "string" && + AI_ERROR_CODES.has(o.code as AiErrorCode) && typeof o.message === "string" && typeof o.status === "number" ); }🤖 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 `@editor/lib/ai/error.ts` around lines 192 - 200, isAiErrorResponse currently accepts any string for the code field; update it to validate that o.code is one of the known AiErrorCode values so malformed payloads like { success: false, code: "foo", ... } fail the guard. Modify isAiErrorResponse to import/use the authoritative AiErrorCode set (or an isAiErrorCode helper) and replace the loose typeof o.code === "string" check with a membership check against AiErrorCode; this ensures downstream functions like resolveAiError and handleAiFetchErrorResponse only see envelopes with a valid code.editor/app/(site)/organizations/[organization_name]/settings/billing/_actions.ts-837-850 (1)
837-850:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same top-up bounds to the direct charge path.
topUpAiCredits()only rejects non-positive values, so any caller that reaches this exported owner action can bypass the documented top-up min/max guardrails and create arbitrarily large direct charges. ReuseTOPUP_MIN_CENTS/TOPUP_MAX_CENTShere beforeaddStripeChargedCommit().Suggested fix
- if (!Number.isFinite(amount_cents) || amount_cents <= 0) { + if ( + !Number.isFinite(amount_cents) || + amount_cents < TOPUP_MIN_CENTS || + amount_cents > TOPUP_MAX_CENTS + ) { throw new BillingError("invalid amount", "invalid_amount", 400); }🤖 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 `@editor/app/`(site)/organizations/[organization_name]/settings/billing/_actions.ts around lines 837 - 850, topUpAiCredits currently only rejects non-positive amounts and therefore allows callers to create arbitrarily large direct charges; update the function topUpAiCredits to validate amount_cents against the existing TOPUP_MIN_CENTS and TOPUP_MAX_CENTS constants before calling addStripeChargedCommit (reuse the same bounds-check logic used elsewhere), throwing a BillingError with the same error codes/message when amount_cents is out of range, so the direct charge path enforces the documented min/max guardrails.editor/app/(insiders)/insiders/billing/actions.ts-49-55 (1)
49-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a runtime dev-only guard inside
wrap().These actions intentionally skip membership checks and accept arbitrary
organizationIds. The only current protection is route-group/proxy placement, but importing from a production code path would expose privileged Stripe/Metronome mutations immediately. A dev-only gate insidewrap()ensures the failure mode is "request rejected" instead of "silent privilege bypass".🤖 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 `@editor/app/`(insiders)/insiders/billing/actions.ts around lines 49 - 55, wrap() currently allows privileged actions unconditionally; add a runtime dev-only guard at the top of wrap() that rejects calls when the code is running outside a developer environment. Concretely: in the wrap(fn) function (which returns ActionResult<T> and uses errorMessage()), check a runtime dev flag (e.g. process.env.NODE_ENV === 'development' or a dedicated runtime flag like globalThis.__INSIDERS_DEV__ or NEXT_PUBLIC_INSIDERS) and if the guard fails return { ok: false, error: 'Insiders-only action attempted in non-dev runtime' } (use errorMessage for consistency if desired) so the request is rejected instead of silently bypassing membership checks. Ensure the check is simple, clearly named, and documented so imports from production paths cannot execute privileged Stripe/Metronome mutations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08a192e3-e313-4ede-8949-a33a3a41b466
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (101)
.agents/skills/database/SKILL.md.agents/skills/security/SKILL.mdSECURITY.mddatabase/database-generated.types.tsdocs/contributing/billing.mddocs/wg/platform/billing/_category_.jsondocs/wg/platform/billing/ai-credits.mddocs/wg/platform/billing/index.mddocs/wg/platform/billing/known-issues.mddocs/wg/platform/billing/metronome.mddocs/wg/platform/index.mdeditor/.env.exampleeditor/.oxlintrc.jsonceditor/app/(api)/internal/cron/billing-reconcile/route.tseditor/app/(api)/private/ai/audio/actions.tseditor/app/(api)/private/ai/audio/generate/route.tseditor/app/(api)/private/ai/chat/route.tseditor/app/(api)/private/ai/credits/route.tseditor/app/(api)/private/ai/generate/image/route.tseditor/app/(api)/private/ai/image/actions.tseditor/app/(api)/private/ai/image/remove-background/route.tseditor/app/(api)/private/ai/image/upscale/route.tseditor/app/(api)/private/ai/models/openai/route.tseditor/app/(api)/private/ai/models/route.tseditor/app/(api)/private/ai/ratelimit.tseditor/app/(api)/private/editor/ai/schema/route.tseditor/app/(api)/private/webhooks/stripe/route.tseditor/app/(canvas)/canvas/tools/ai/_hooks/use-models.tseditor/app/(canvas)/canvas/tools/ai/generate.tseditor/app/(ingest)/README.mdeditor/app/(ingest)/webhooks/metronome/route.tseditor/app/(ingest)/webhooks/stripe/route.tseditor/app/(insiders)/insiders/billing/_view.tsxeditor/app/(insiders)/insiders/billing/actions.tseditor/app/(insiders)/insiders/billing/page.tsxeditor/app/(insiders)/layout.tsxeditor/app/(site)/organizations/[organization_name]/settings/billing/_actions.tseditor/app/(site)/organizations/[organization_name]/settings/billing/_view.tsxeditor/app/(site)/organizations/[organization_name]/settings/billing/return/_view.tsxeditor/app/(site)/organizations/[organization_name]/settings/billing/return/page.tsxeditor/app/(tools)/(playground)/playground/image/_page.tsxeditor/app/(tools)/(playground)/playground/image/layout.tsxeditor/app/(tools)/tools/remove-bg/_page.tsxeditor/app/(www)/(ai)/ai/_page.tsxeditor/app/(www)/(ai)/ai/music/page.tsxeditor/app/(www)/(ai)/ai/music/playground/_page.tsxeditor/app/(www)/(ai)/ai/page.tsxeditor/app/(www)/(ai)/layout.tsxeditor/grida-canvas-hosted/ai/agent/server-agent.tseditor/grida-canvas-hosted/ai/tools/canvas-use.tseditor/grida-canvas-react-starter-kit/starterkit-toolbar/image-toolbar.tsxeditor/lib/__tests__/server-only.shim.tseditor/lib/ai/__tests__/error.test.tseditor/lib/ai/__tests__/server.test.tseditor/lib/ai/actions/audio.tseditor/lib/ai/actions/chat.tseditor/lib/ai/actions/forms-schema.tseditor/lib/ai/actions/image-generate.tseditor/lib/ai/actions/image.tseditor/lib/ai/actions/models.tseditor/lib/ai/ai.tseditor/lib/ai/credits/README.mdeditor/lib/ai/credits/__tests__/controller.test.tseditor/lib/ai/credits/__tests__/format.test.tseditor/lib/ai/credits/actions.tseditor/lib/ai/credits/controller.tseditor/lib/ai/credits/format.tseditor/lib/ai/credits/index.tseditor/lib/ai/credits/provider.tsxeditor/lib/ai/error.tseditor/lib/ai/hooks/index.tseditor/lib/ai/hooks/use-credits.tseditor/lib/ai/hooks/use-generate-audio.tseditor/lib/ai/hooks/use-generate-image.tseditor/lib/ai/models.tseditor/lib/ai/server.tseditor/lib/auth/organization.tseditor/lib/billing/__tests__/e2e/fixtures/db.tseditor/lib/billing/__tests__/e2e/fixtures/deliver-event.tseditor/lib/billing/__tests__/e2e/fixtures/safety.tseditor/lib/billing/__tests__/e2e/scenarios/ai-credit-topup.test.tseditor/lib/billing/fees.tseditor/lib/billing/metronome.tseditor/lib/billing/plans.tseditor/package.jsoneditor/proxy.tseditor/scaffolds/ai/form-field-schema-assistant.tsxeditor/scaffolds/playground-forms/actions.tseditor/scaffolds/playground-forms/playground.tsxeditor/scripts/audit-ai-seam.tseditor/scripts/billing/README.mdeditor/scripts/billing/_env.tseditor/scripts/billing/cli.tseditor/scripts/billing/ops.tseditor/scripts/billing/setup-stripe-test.tseditor/scripts/billing/setup.tseditor/scripts/billing/smoke.tseditor/vercel.jsoneditor/vitest.config.tssupabase/migrations/20260508130000_grida_billing_metronome.sqlsupabase/schemas/grida_billing.sql
💤 Files with no reviewable changes (18)
- editor/lib/ai/hooks/index.ts
- editor/app/(api)/private/ai/audio/actions.ts
- editor/app/(api)/private/ai/models/openai/route.ts
- editor/app/(api)/private/ai/models/route.ts
- editor/lib/ai/hooks/use-credits.ts
- editor/app/(api)/private/ai/image/remove-background/route.ts
- editor/app/(api)/private/ai/audio/generate/route.ts
- editor/app/(api)/private/ai/image/upscale/route.ts
- editor/lib/ai/hooks/use-generate-image.ts
- editor/lib/ai/hooks/use-generate-audio.ts
- editor/lib/billing/plans.ts
- editor/app/(api)/private/ai/credits/route.ts
- editor/app/(api)/private/ai/ratelimit.ts
- editor/app/(api)/private/editor/ai/schema/route.ts
- editor/app/(api)/private/webhooks/stripe/route.ts
- editor/app/(api)/private/ai/image/actions.ts
- editor/scripts/billing/setup-stripe-test.ts
- editor/app/(api)/private/ai/generate/image/route.ts
`service_role.<schema>` was constructed at module-eval time — eleven `createClient` calls fired the moment anything imported `lib/supabase/server.ts`. That made the import contingent on `SUPABASE_SECRET_KEY` being present, which is true in dev (.env.local) but false in CI. Any consumer that transitively reached `service_role` inherited the brittleness: the failure surfaced as unrelated test files crashing on import with `supabaseKey is required`. The recent `lib/ai/credits/__tests__/controller.test.ts` hit it via the `controller → actions → @/lib/ai/server → @/lib/supabase/server` chain. Convert each `service_role.<schema>` to a memoized getter on a const object. The call-site syntax is unchanged (`service_role.workspace.from(...)`), so every consumer keeps working without modification, and the "grep `service_role` to find every privileged DB touch" property is preserved. First access constructs and caches; further accesses return the same client. Lock the contract in `lib/supabase/__tests__/server.test.ts`: - import does not call `createClient` - first property access constructs once - repeat access does not reconstruct (memoized identity) - per-schema construction is independent Revert the test-side mock workaround in `lib/ai/credits/__tests__/controller.test.ts` — the controller test is back to the minimal form (no `vi.mock` for supabase/billing/auth) and passes under empty env (`SUPABASE_SECRET_KEY=""` etc.) because the import chain no longer touches the supabase factory.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
editor/lib/supabase/server.ts (1)
160-194: ⚡ Quick winHarden
service_roleagainst runtime mutation.At Line 160,
service_roleis exported as a plain object with configurable getters. Freezing it prevents accidental/intentional redefinition of privileged accessors.Suggested patch
-export const service_role = { +export const service_role = Object.freeze({ get workspace() { return _workspace(); }, @@ get www() { return _www(); }, -}; +});🤖 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 `@editor/lib/supabase/server.ts` around lines 160 - 194, Wrap the exported service_role object in Object.freeze to prevent runtime mutation of its accessor properties (so the getters like workspace, ciam, library, forms, storage, canvas, sites, commerce, west_referral, xsb, and www cannot be redefined); replace the current plain object export of service_role with a frozen object (e.g., export const service_role = Object.freeze({ ... })) so the property descriptors become non-configurable and the object cannot be mutated at runtime.
🤖 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.
Nitpick comments:
In `@editor/lib/supabase/server.ts`:
- Around line 160-194: Wrap the exported service_role object in Object.freeze to
prevent runtime mutation of its accessor properties (so the getters like
workspace, ciam, library, forms, storage, canvas, sites, commerce,
west_referral, xsb, and www cannot be redefined); replace the current plain
object export of service_role with a frozen object (e.g., export const
service_role = Object.freeze({ ... })) so the property descriptors become
non-configurable and the object cannot be mutated at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b48654dd-5cf3-412e-ba95-8bcd07f4c259
📒 Files selected for processing (2)
editor/lib/supabase/__tests__/server.test.tseditor/lib/supabase/server.ts
The Stripe webhook receiver runs the projector RPC (`fn_billing_apply_stripe_event`) FIRST, which records the event in `grida_billing.stripe_event` and stamps `processed_at`. The AI-credit Checkout post-processor (`handleAiCreditCheckoutCompleted`) ran AFTER. If the post-processor 500'd (Metronome network blip, etc.), the request returned 500 → Stripe retries → projector returns `replayed` → route short-circuited before the post-processor ever ran again. Customer paid; no balance. The post-processor cannot fold into the SQL projector because it makes HTTP calls to Metronome (`addStripeChargedCommit`, `setAutoReload`). Add a separate per-event marker that the receiver consults independently of the projector's replayed/handled distinction. - Migration `20260512000000_grida_billing_stripe_event_ai_credit_marker.sql` adds `ai_credit_processed_at timestamptz` to `grida_billing.stripe_event`, plus `fn_billing_get_ai_credit_processed` (read) and `fn_billing_stamp_ai_credit_processed` (UPSERT) RPCs. Both SECURITY DEFINER, search_path locked, EXECUTE granted only to service_role. Schema mirror in `supabase/schemas/grida_billing.sql` updated to match. - `lib/billing/index.ts` exposes `readAiCreditMarker` and `stampAiCreditMarker` helpers; `as never` casts mirror the existing `fn_billing_get_metronome_account` pattern (generated DB types lag newly-added RPCs until `pnpm typecheck` is re-run after the migration applies locally). - `app/(ingest)/webhooks/stripe/route.ts` now runs the AI-credit branch on every `checkout.session.completed` delivery — gated on `readAiCreditMarker(event.id) === true`, not on `result.result`. Marker stamped after the post-processor returns successfully (including `noop`, so non-AI-credit checkouts also short-circuit cleanly on replay). On failure the marker is NOT stamped — Stripe retries, the replay sees marker NULL, re-enters the branch. - `replayed` short-circuit moved AFTER the AI-credit branch so it still fires for non-checkout replays. - Doc comment block at top updated to describe the split-marker design. The existing e2e test (`lib/billing/__tests__/e2e/scenarios/ai-credit-topup.test.ts`) still passes — it calls `handleAiCreditCheckoutCompleted` directly, bypassing the receiver and the marker.
CodeRabbit + ChatGPT Codex flagged ~25 issues against `1e4134eb`. After
fact-checking each against current code (~9 turned out STALE/WRONG —
Bearer auth already supported, payment_status already validated, bounds
already enforced one layer down, etc.), 15 narrow fixes landed here.
**AI seam — error envelope (`lib/ai/error.ts`)**
- Strict `AiErrorCode` union: codes are now an `AI_ERROR_CODES as const`
array with the union derived from it. Per-code JSDoc inline. Adding a
new code requires touching the array, the typeguard, the switch, and
the tests in lock-step (TS exhaustiveness + `assertNever` enforce it).
- Sanitize internal-path messages: `billingErrorToAiError` no longer
echoes raw exception `.message` to clients on the `internal` path —
replaced with a constant; raw error still goes to `console.error` for
debugging. The `blocked` path is unchanged because its messages are
curated for end-user display.
- Propagate real DB faults: `orgErrorToAiError` no longer collapses
`org_lookup_failed` (and unknown codes) into `no_organization`.
Recoverable codes keep their typed mapping; everything else re-throws
so the caller's outer try/catch (route or framework) handles it as a
generic 500. Per project policy: real DB exceptions should NOT be
wrapped in a misleading user-facing redirect.
- Strict `isAiErrorResponse`: rejects envelopes whose `code` isn't in
the union. Closes the previously silent fall-through where an unknown
code returned `undefined` and callers treated it as "no action."
- `resolveAiError` switch is closed with an `assertNever` default.
**AI seam — server (`lib/ai/server.ts`)**
- `extractContext`: preserve `awaitIngest: undefined` so
`withTransaction`'s `?? true` default applies. Coercing to `false`
silently flipped non-streaming AI calls to fire-and-forget ingest,
letting `withAiAuth`'s post-call balance read race the debit.
**AI seam — actions (`lib/ai/actions/*`, `app/(api)/private/ai/chat/route.ts`)**
- `chat.ts`: derive `resolvedId` from `catalog[tiers.mini].id` so a
future `tiers.mini` remap can't desync the billed cost card from the
SDK call (was hardcoded `"openai/gpt-5.4-mini"`).
- `image.ts (upscaleImage)`: bound-check `scale` (finite number, 1-10)
before forwarding — Replicate's real-esrgan rejects out-of-range with
cryptic errors.
- `chat/route.ts`: 500 path now returns the typed `AiActionError`
envelope via `aiErrorResponse(...)` instead of plain `"Internal error"`
text, so client-side `consume()` can route it.
**AI seam — credits (`lib/ai/credits/actions.ts`)**
- `preloadAiCredits`: cache-first read via `getEntitlement` (hits the
local `grida_billing.account` cache) instead of also calling
`refreshBalance` (live Metronome read on every layout render). Aligns
with the team's "billing webhook is sole source of truth" rule —
webhooks + the post-action `withAiAuth` envelope keep the cache fresh,
and `useAiCredits().refresh()` covers manual re-sync.
**AI seam — lint (`.oxlintrc.jsonc`)**
- Extend `no-restricted-imports` `patterns` so subpath imports of
provider SDKs (`openai/resources`, `@anthropic-ai/sdk/foo`,
`replicate/lib/...`, `@ai-sdk/openai/internal`, `stripe/lib/...`) also
fail the lint. Bare-name imports were already blocked via `paths`.
Verified subpath blocking via a scratch file; zero false positives
across the existing tree (1010 files).
**Billing — service layer + webhook**
- `billing/metronome.ts (getAccountView)`: compare
`db.cached_balance_cents !== Math.floor(live.balanceCents)` — the
cache is written floored, so a fractional live value triggered
spurious "drift" warnings on every read.
- `webhooks/metronome/route.ts`: symmetric ±5 min freshness window. The
previous `Date.now() - dateMs > FIVE_MINUTES_MS` rejected stale events
but accepted arbitrary future timestamps (clock-skew / forgery defense).
**Insiders — security defense in depth**
- `(insiders)/insiders/billing/actions.ts (wrap)`: runtime
`NODE_ENV !== "development"` guard. GRIDA-SEC-002 is enforced at the
proxy + (insiders) layout, but server-action hashes are addressable
from any importing page; the guard ensures the unauth mutators refuse
to execute even if an import accidentally crosses the boundary.
**Hosted canvas — LLM tool input validation**
- `grida-canvas-hosted/ai/tools/canvas-use.ts`: tighten the LLM tool
schema with `.regex(/^\d+:\d+$/)` on `aspect_ratio` so the AI SDK
rejects malformed values at validation time with a clear error the
LLM can self-correct, instead of forwarding garbage to the image
provider and surfacing an opaque downstream failure.
**Starterkit — host-injected organization id**
- New `<StarterKitOrgIdProvider>` + `useStarterKitOrgId()` hook (mirrors
the existing `PreviewProvider` / `usePreview` pattern in
`starterkit-preview/`). The starterkit stays workspace-agnostic; the
host (`CanvasPlayground`, slides page) injects the verified
organization id. `<ImageToolbar />`'s upscale / remove-background
buttons now thread it into the AI seam actions; null org → friendly
toast ("Sign in to use AI tools") instead of letting the server
return 400 (GRIDA-SEC-003).
**UX — orphan / stuck states**
- `scaffolds/playground-forms/playground.tsx`: restructure
`generate(...).then(...)` to `.then().catch().finally()` so the
"Generating…" state always clears, even when the action rejects.
- `app/(tools)/(playground)/playground/image/_page.tsx`: wrap the
generation flow in try/catch and `editor.commands.delete([id])` on
both gate-skip and thrown error — previously a failed generate left
an empty image node on the canvas.
Tests: 70/70 in `lib/ai` + `lib/supabase` (added 11 new tests covering
`isAiErrorCode`, the re-throw semantics for `org_lookup_failed` /
unknown codes, `billingErrorToAiError`'s sanitization, and the strict
typeguard).
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
editor/lib/ai/__tests__/error.test.ts (1)
10-20: ⚡ Quick winAdd focused tests for
handleAiFetchErrorResponsebranch behavior.This file validates most seam helpers, but the fetch helper’s redirect-null short-circuit and malformed JSON fallback paths are still untested.
🧪 Suggested test additions
import { aiErrorResponse, AI_ERROR_CODES, billingErrorToAiError, + handleAiFetchErrorResponse, isAiErrorCode, isAiErrorResponse, orgErrorToAiError, resolveAiError, type AiErrorResponse, } from "../error";+describe("handleAiFetchErrorResponse", () => { + it("redirect action returns null after navigation", async () => { + const origWindow = globalThis.window; + Object.defineProperty(globalThis, "window", { + configurable: true, + value: { location: { href: "" } }, + }); + + const res = new Response( + JSON.stringify({ + success: false, + code: "unauthorized", + message: "login required", + status: 401, + }), + { status: 401, headers: { "content-type": "application/json" } } + ); + + const out = await handleAiFetchErrorResponse(res); + expect(out).toBeNull(); + expect(globalThis.window.location.href).toContain("/sign-in?next="); + + if (origWindow === undefined) { + delete (globalThis as { window?: unknown }).window; + } else { + Object.defineProperty(globalThis, "window", { + configurable: true, + value: origWindow, + }); + } + }); + + it("falls back to internal message when payload is not JSON", async () => { + const res = new Response("not-json", { status: 500 }); + const out = await handleAiFetchErrorResponse(res); + expect(out).toEqual({ + message: "Something went wrong. Please try again.", + }); + }); +});Also applies to: 291-329
🤖 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 `@editor/lib/ai/__tests__/error.test.ts` around lines 10 - 20, Add two focused tests in editor/lib/ai/__tests__/error.test.ts exercising handleAiFetchErrorResponse: one that stubs a fetch Response with redirected=true and url set to the literal "null" to verify the function takes the redirect-null short-circuit path and returns the expected AiErrorResponse shape; and a second that stubs a Response whose json() either throws or returns malformed data to verify handleAiFetchErrorResponse falls back to the generic error branch (using response.status/statusText) rather than blowing up. Use the existing aiErrorResponse, isAiErrorResponse, and AI_ERROR_CODES utilities to assert the returned objects.
🤖 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 `@editor/lib/ai/error.ts`:
- Around line 298-299: The current return uses (data as { message?: string
}).message without type checking, which can leak non-string values to callers;
update the fallback logic in this function to read data.message, verify typeof
message === "string" (and optionally non-empty), and only return it if valid,
otherwise return INTERNAL_ERROR_MESSAGE; reference the existing
variable/assignment for fallback and the final returned object { message: ... }
so you replace the cast with a guarded check against data.message and fallback
to INTERNAL_ERROR_MESSAGE when the type is not a string.
- Line 165: Replace the raw error dump at the console.error call that currently
logs `[${scope}]` and the entire err object with a sanitized log containing only
scope and selected harmless fields (e.g., err.code and err.status); locate the
console.error(`[${scope}]`, err) usage in editor/lib/ai/error.ts and change it
to log a structured message like scope plus { code, status } (avoid logging
err.stack or the whole err object) so provider/internal details are not
persisted.
In `@editor/lib/billing/index.ts`:
- Around line 453-462: The function stampAiCreditMarker currently only logs RPC
errors (in stampAiCreditMarker) which swallows failures; change it to propagate
the error so callers can handle retries/failures: after calling
service_role.workspace.rpc in stampAiCreditMarker, if the returned error is
truthy, throw that error (or throw a new Error with contextual message that
includes error.message) instead of only console.warn, ensuring the dedup marker
failure bubbles up to the caller for explicit handling.
---
Nitpick comments:
In `@editor/lib/ai/__tests__/error.test.ts`:
- Around line 10-20: Add two focused tests in
editor/lib/ai/__tests__/error.test.ts exercising handleAiFetchErrorResponse: one
that stubs a fetch Response with redirected=true and url set to the literal
"null" to verify the function takes the redirect-null short-circuit path and
returns the expected AiErrorResponse shape; and a second that stubs a Response
whose json() either throws or returns malformed data to verify
handleAiFetchErrorResponse falls back to the generic error branch (using
response.status/statusText) rather than blowing up. Use the existing
aiErrorResponse, isAiErrorResponse, and AI_ERROR_CODES utilities to assert the
returned objects.
🪄 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
Run ID: 03e9cffd-ceef-40fb-9789-e2b1277d3dd7
📒 Files selected for processing (23)
editor/.oxlintrc.jsonceditor/app/(api)/private/ai/chat/route.tseditor/app/(canvas)/canvas/slides/page.tsxeditor/app/(ingest)/webhooks/metronome/route.tseditor/app/(ingest)/webhooks/stripe/route.tseditor/app/(insiders)/insiders/billing/actions.tseditor/app/(tools)/(playground)/playground/image/_page.tsxeditor/grida-canvas-hosted/ai/tools/canvas-use.tseditor/grida-canvas-hosted/playground/playground.tsxeditor/grida-canvas-react-starter-kit/starterkit-host/org-id-provider.tsxeditor/grida-canvas-react-starter-kit/starterkit-toolbar/image-toolbar.tsxeditor/lib/ai/__tests__/error.test.tseditor/lib/ai/actions/chat.tseditor/lib/ai/actions/image.tseditor/lib/ai/credits/actions.tseditor/lib/ai/error.tseditor/lib/ai/server.tseditor/lib/billing/index.tseditor/lib/billing/metronome.tseditor/scaffolds/playground-forms/playground.tsxlefthook.ymlsupabase/migrations/20260512000000_grida_billing_stripe_event_ai_credit_marker.sqlsupabase/schemas/grida_billing.sql
✅ Files skipped from review due to trivial changes (1)
- lefthook.yml
🚧 Files skipped from review as they are similar to previous changes (14)
- editor/app/(api)/private/ai/chat/route.ts
- editor/grida-canvas-hosted/ai/tools/canvas-use.ts
- editor/lib/ai/credits/actions.ts
- editor/app/(ingest)/webhooks/stripe/route.ts
- editor/lib/ai/actions/chat.ts
- editor/scaffolds/playground-forms/playground.tsx
- editor/app/(ingest)/webhooks/metronome/route.ts
- editor/app/(tools)/(playground)/playground/image/_page.tsx
- editor/.oxlintrc.jsonc
- editor/lib/ai/actions/image.ts
- editor/grida-canvas-react-starter-kit/starterkit-toolbar/image-toolbar.tsx
- editor/lib/billing/metronome.ts
- editor/app/(insiders)/insiders/billing/actions.ts
- editor/lib/ai/server.ts
| message: err instanceof Error ? err.message : "blocked", | ||
| }; | ||
| } | ||
| console.error(`[${scope}]`, err); |
There was a problem hiding this comment.
Avoid logging raw billing errors directly.
Line 165 logs the full err object, which can persist provider/internal details in logs. Prefer logging sanitized fields only (scope, code, status) and avoid raw stack/object dumps by default.
🔧 Suggested change
- console.error(`[${scope}]`, err);
+ console.error(`[${scope}] billing_error`, {
+ code: errorCode(err) ?? "unknown",
+ status: extractStatus(err, 500),
+ kind: err instanceof Error ? err.name : typeof err,
+ });🤖 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 `@editor/lib/ai/error.ts` at line 165, Replace the raw error dump at the
console.error call that currently logs `[${scope}]` and the entire err object
with a sanitized log containing only scope and selected harmless fields (e.g.,
err.code and err.status); locate the console.error(`[${scope}]`, err) usage in
editor/lib/ai/error.ts and change it to log a structured message like scope plus
{ code, status } (avoid logging err.stack or the whole err object) so
provider/internal details are not persisted.
Comment on lines
+298
to
+299
| const fallback = (data as { message?: string }).message; | ||
| return { message: fallback ?? INTERNAL_ERROR_MESSAGE }; |
There was a problem hiding this comment.
Validate fallback message type before returning UI text.
Line 298 currently trusts data.message via a cast; non-string payloads can bleed through to callers expecting a string.
🔧 Suggested change
- const fallback = (data as { message?: string }).message;
- return { message: fallback ?? INTERNAL_ERROR_MESSAGE };
+ const fallback = (data as { message?: unknown }).message;
+ return {
+ message:
+ typeof fallback === "string" && fallback.length > 0
+ ? fallback
+ : INTERNAL_ERROR_MESSAGE,
+ };📝 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.
Suggested change
| const fallback = (data as { message?: string }).message; | |
| return { message: fallback ?? INTERNAL_ERROR_MESSAGE }; | |
| const fallback = (data as { message?: unknown }).message; | |
| return { | |
| message: | |
| typeof fallback === "string" && fallback.length > 0 | |
| ? fallback | |
| : INTERNAL_ERROR_MESSAGE, | |
| }; |
🤖 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 `@editor/lib/ai/error.ts` around lines 298 - 299, The current return uses (data
as { message?: string }).message without type checking, which can leak
non-string values to callers; update the fallback logic in this function to read
data.message, verify typeof message === "string" (and optionally non-empty), and
only return it if valid, otherwise return INTERNAL_ERROR_MESSAGE; reference the
existing variable/assignment for fallback and the final returned object {
message: ... } so you replace the cast with a guarded check against data.message
and fallback to INTERNAL_ERROR_MESSAGE when the type is not a string.
Comment on lines
+453
to
+462
| export async function stampAiCreditMarker( | ||
| eventId: string, | ||
| eventType: string | ||
| ): Promise<void> { | ||
| const { error } = await service_role.workspace.rpc( | ||
| "fn_billing_stamp_ai_credit_processed" as never, | ||
| { p_event_id: eventId, p_event_type: eventType } as never | ||
| ); | ||
| if (error) console.warn("[billing] stamp_ai_credit_marker:", error.message); | ||
| } |
There was a problem hiding this comment.
Do not swallow AI-credit marker stamp failures.
On Line 461, warning-only handling can acknowledge the flow without persisting the dedup marker, which can re-trigger post-processing on replay paths. Bubble this failure so the caller can decide retry/fail behavior explicitly.
Suggested fix
export async function stampAiCreditMarker(
eventId: string,
eventType: string
): Promise<void> {
const { error } = await service_role.workspace.rpc(
"fn_billing_stamp_ai_credit_processed" as never,
{ p_event_id: eventId, p_event_type: eventType } as never
);
- if (error) console.warn("[billing] stamp_ai_credit_marker:", error.message);
+ if (error) {
+ throw new Error(`stamp_ai_credit_marker: ${error.message}`);
+ }
}🤖 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 `@editor/lib/billing/index.ts` around lines 453 - 462, The function
stampAiCreditMarker currently only logs RPC errors (in stampAiCreditMarker)
which swallows failures; change it to propagate the error so callers can handle
retries/failures: after calling service_role.workspace.rpc in
stampAiCreditMarker, if the returned error is truthy, throw that error (or throw
a new Error with contextual message that includes error.message) instead of only
console.warn, ensuring the dedup marker failure bubbles up to the caller for
explicit handling.
The billing CLI is for one-off, human-driven runs against real external services. Auto-loading `.env.test.local` / `.env.test` / `.env.local` from cwd as a side-effect import made it too easy to fire commands against the wrong tenant — the prod cutover for PR #711 is the forcing function to fix this. Three guards now: 1. **`--env=<spec>` is required.** No env file is auto-discovered. The loader is a function (`loadEnvSpec`) called explicitly by `cli.ts` after flag parsing, not a top-level side-effect import. Sibling modules (`setup.ts`, `smoke.ts`, `ops.ts`) drop their `import "./_env"` and trust the caller to populate `process.env`. --env=dev load editor/.env.test.local etc. --env=/abs/path/to/file load exactly that file --env=~/foo/file leading `~/` is expanded to $HOME (shells only expand `~` at the start of unquoted words, so this matters) 2. **Typed confirmation prompt.** After loading, the CLI prints the loaded env file paths, every relevant env var (redacted with length — `sk_test_*** (len=107)` so the user can sanity-check it's the right token), and the action that's about to run. The user then types the exact command name (`ping`, `setup:metronome`, etc.) to proceed. Empty / mismatched input aborts. 3. **Live-Stripe override.** If `STRIPE_SECRET_KEY` starts with `sk_live_`, the prompt requires typing `PROD` instead of the command name and prints a "STRIPE LIVE MODE detected" warning. Belt-and- suspenders — `requireStripeTestKey()` already refuses live keys for Stripe-touching commands; this is the catch-all for non-Stripe commands (Metronome, backfill) that nonetheless mutate prod state. Failure modes are explicit: - Missing `--env` → exit 2 with usage. - Bad env path → "file not found". - Unknown command → exit 2 with usage. - Confirmation mismatch → exit 1, no command runs. README rewritten to document the new contract + a recommended prod file layout (env file outside the repo, `chmod 600`, no `STRIPE_SECRET_KEY` since the CLI refuses live keys anyway). Smoke-tested manually: `--help`, missing `--env`, bad path, dev-target prompt, leading `~/` expansion — all behave as expected. `pnpm exec tsc --noEmit` clean.
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
1 participant
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.
Summary
Two cohesive halves under one PR:
app/(api)/private/ai/*route handlers into a singlelib/ai/{server,actions,credits}module that goes through the new credit gate; ship a public/aichat page on top.Webhook receivers (Stripe + new Metronome) live under the
(ingest)route group introduced in this PR's first commit (6604d6c43) — see GRIDA-SEC-001 in SECURITY.md for the trust-boundary contract.Roles in the final architecture:
customer_entitled)lib/ai/server.ts= the only file allowed to import provider SDKs; every call goes throughwithAiAuth(membership + entitlement + balance gate)What ships — billing v1
grida_billing.accountgains Metronome ids + entitlement / cache / auto-reload columns; newmetronome_eventdedup table; new projector RPC (fn_billing_apply_metronome_event) — multi-tier-alert aware (only depletion-tier flips entitlement off; warning tiers refresh balance only).editor/lib/billing/metronome.ts(~1400 LOC) — provision, commits, ingest, auto-reload, gate, reconcile, invoices, refund. Flat markup envelopeceil((credit + 30) / 0.95)infees.ts, verified safe across every Stripe card type./webhooks/metronome(HMAC + dedup + projector). Existing/webhooks/stripeextended with two AI-credit branches (post-Checkout reconciliation + subscription-cancel → disable auto-reload)./internal/cron/billing-reconcilesweeps only provisioned orgs./insiders/billingexercises every Metronome primitive directly. Gated by GRIDA-SEC-002 (proxy 404 + layoutnotFound()) so the dev-only mutators can never reach prod.editor/scripts/billing/cli.ts:setup:stripe,setup:metronome,ping,smoke:{topup,auto-reload,webhook},backfill,markup-sim.ai-credit-topup.test.ts) driveshandleAiCreditCheckoutCompleteddirectly against the Metronome sandbox — no browser, no real Checkout, no dev server.docs/wg/platform/billing/—ai-credits.md(master plan),metronome.md(integration),known-issues.md(living KI-BILL register). Contributor setup atdocs/contributing/billing.mdrewritten for the CLI + locally-configured cloudflared tunnel.What ships — AI seam + /ai page
editor/lib/auth/organization.ts— single producer of verifiedorganizationIdfor AI callers. Resolves route slug → header → explicit input → session-derived org, with membership established at every step. The session resolver intersectsuser_project_access_statewithpublic.get_organizations_for_user(the SECURITY DEFINER membership primitive), so a stale access-state row left over after a membership revocation can never escalate into another org's billing context. SECURITY.md documents the boundary.editor/lib/ai/server.ts— single seam entry point, the only file allowed to import provider SDKs (Vercel AI SDK / Replicate / OpenAI / Anthropic). Enforced byoxlint no-restricted-imports+editor/scripts/audit-ai-seam.ts(CI). Wraps every call inwithAiAuth(membership + entitlement + balance gate).editor/lib/ai/error.ts— typedAiActionErrorenvelope shared by every action.editor/lib/ai/actions/{chat,audio,image,image-generate,forms-schema,models}.tsreplace the deleted route handlers underapp/(api)/private/ai/*. None re-implements auth — all go throughwithAiAuth.editor/lib/ai/credits/— controller (refresh / debit / format),<AiCredits.Provider>+useAiCredits()hook, server actions (preloadAiCredits,resolveInitialAiCredits,refreshAiCredits). Replaces ad-hocuse-credits/use-generate-*hooks. Backed by tests for the controller state machine and the cents formatter.ai/generate,use-models), playground/image, tools/remove-bg, ai/music, scaffolds/ai (form-field schema assistant), scaffolds/playground-forms, grida-canvas-hosted (server-agent + canvas-use), starterkit image toolbar./aichatapp/(www)/(ai)/{layout,ai/page,ai/_page}.tsx— minimal user-facing chat surface that flexes the new seam end to end. Auth gate at submit time (intentionally not pre-gated); credit chip + balance preload via the route-group layout's<AiCredits.Provider>; session-resolved org for the credit context.editor/lib/__tests__/server-only.shim.ts+vitest.config.tschange so server-only modules load under vitest withoutimport \"server-only\"blowing up. New tests:lib/ai/__tests__/{error,server}.test.ts,lib/ai/credits/__tests__/{controller,format}.test.ts.Commits
697185a1cfeat(billing): add Metronome AI credit service layere79107b48feat(billing): Metronome + Stripe ai-credit webhooks and reconcile cron2b535c695feat(billing): user-facing AI credit page (top-up + auto-reload)ec9a643b2feat(insiders): billing QA harness + GRIDA-SEC-002 dev-only gateb4d31f199chore(scripts/billing): consolidate dev/QA scripts into one CLIec1bf8b23test(billing): headless e2e for AI-credit topup post-Checkout flow6c3cb1679docs(billing): AI credit design, Metronome integration, known issuesfe5256973refactor(docs): group billing WG docs under wg/platform/billing/36e98cb4cdocs(wg/billing): fix bullet rendering in index.md2a603fc1fdocs(wg/billing): escape \<100 ms` to fix MDX build`863c0ef09docs(billing): add known issue for plan-included credit not grantedc34b205b2feat(security): GRIDA-SEC-003 — verified org-id resolver3837b06f2refactor(ai): consolidate API routes into lib/ai server actions + credits module1e4134eb8feat(ai): public /ai chat pageMoney model & known issues
lib/billing/fees.ts): flat 5% + $0.30 gross-up, single safe envelope across US / intl / AmEx cards + 1% FX. Applied to manual top-up Checkout, initial auto-reload setup, and all silent recharges (payment_type: INVOICE). Audit trail:cli.ts markup-sim.invoice_creation: { enabled: true }on Checkout sessions andpayment_type: \"INVOICE\"on Metronome's threshold config.Full register:
docs/wg/platform/billing/known-issues.md.Verification
/aichat exercised end-to-end with credit chip refreshOperational checklist (for prod cutover)
Test plan
Summary by CodeRabbit
New Features
Documentation
Tests / Tools