feat: root Supabase + Paystack billing v2, SME mobile updates, announcements & feedback tooling - #34
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCentralizes Supabase runtime/migrations under Changes
Sequence Diagram(s)sequenceDiagram
participant Mobile as Mobile App
participant Edge as Supabase Edge Function
participant DB as Supabase DB
participant Paystack as Paystack API
Note over Mobile,Paystack: MoMo charge (idempotent async flow)
Mobile->>Edge: POST /paystack-momo-charge { plan_key, idempotency_key, phone }
Edge->>DB: SELECT momo_charge_attempts WHERE idempotency_key,user_id
alt existing attempt
DB-->>Edge: cached attempt
Edge-->>Mobile: return cached outcome (pending|success|failed)
else new attempt
Edge->>DB: INSERT momo_charge_attempts (status=pending)
Edge->>Paystack: POST /charge (mobile_money payload)
Paystack-->>Edge: response (accepted|failed|success)
Edge->>DB: UPDATE momo_charge_attempts (status, paystack_reference, paystack_response)
Edge-->>Mobile: return outcome + reference
end
Note over Paystack,Edge: Webhook reconciliation (dedupe & subscription updates)
Paystack->>Edge: POST /paystack-webhook (event)
Edge->>Edge: verify HMAC signature
Edge->>DB: INSERT paystack_applied_charges (dedupe) / UPSERT subscriptions / INSERT subscription_activity_log
Edge-->>Paystack: 200 OK
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
❌ TypeScript errors ( |
![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: a09a606ca3
ℹ️ 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".
| # Allow dynamic client registration | ||
| allow_dynamic_registration = false | ||
|
|
||
| [edge_runtime] |
There was a problem hiding this comment.
This repo-level config.toml never defines a [functions.paystack-webhook] block, so the function keeps the default JWT gate even though paystack-webhook is designed for third-party callbacks (it validates x-paystack-signature itself). In a normal Supabase deploy, Paystack requests will be rejected before reaching this handler because they do not carry a Supabase user JWT, which means payment events are dropped.
Useful? React with 👍 / 👎.
| default: | ||
| console.log(`${WEBHOOK_LOG} Unhandled event`, { | ||
| eventType, | ||
| preview: safePreview(event.data, 400), | ||
| }); |
There was a problem hiding this comment.
The webhook switch handles charge.failed but does not handle invoice.payment_failed, and unrecognized events fall through to a no-op in default. For subscription renewals, failed charges are delivered via invoice failure events, so these failures will not transition subscriptions into grace_period, leaving access state stale until some unrelated event occurs.
Useful? React with 👍 / 👎.
| if (reference) { | ||
| const { data: already, error: idemErr } = await admin | ||
| .from("paystack_applied_charges") | ||
| .select("reference") | ||
| .eq("reference", reference) | ||
| .maybeSingle(); |
There was a problem hiding this comment.
The duplicate check reads paystack_applied_charges first, but the reference is inserted only after subscription updates complete. If Paystack retries quickly (or two workers process the same event concurrently), both executions can pass the pre-check and both mutate subscription state, causing duplicate lifecycle writes and period recalculation. Make the idempotency write atomic (insert-first/upsert in a transaction) before applying state changes.
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: 59
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/.gitignore (1)
5-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIgnore base
.envvariants to prevent accidental secret commits.Line 7-Line 8 ignore only local variants;
supabase/.envand other non-local env files remain trackable.Suggested patch
# dotenvx +.env +.env.* +!.env.example .env.keys .env.local .env.*.local🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/.gitignore` around lines 5 - 9, The .gitignore currently only ignores .env.local, .env.keys and .env.*.local leaving supabase/.env and other non-local env files trackable; update supabase/.gitignore to also ignore the base environment files by adding entries for .env and .env.* (while preserving any example files like .env.example), so that .env and any non-local variants are not committed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 28-29: The current .gitignore entry "backups/" prevents unignoring
a file inside that directory; replace the directory-wide ignore with a pattern
that ignores files but allows exceptions: remove or change "backups/" to
"backups/*" and keep or add the exception "!backups/README.md" (and if needed
add "!backups/" before the file exception) so the README can be re-included;
target the "backups/" and "backups/README.md" entries when making this change.
In `@cediwise-mobile-app/utils/supabase.ts`:
- Around line 23-35: nativeAuthLock currently ignores the acquireTimeout
parameter and can block indefinitely; update nativeAuthLock to honor the
_acquireTimeout by racing the queued acquisition (nativeAuthMutex.then(() =>
fn())) against a timeout Promise that rejects when elapsed with an Error object
that has error.isAcquireTimeout === true, ensure nativeAuthMutex is still
advanced/cleared on both resolve and reject (so later callers aren't permanently
stuck), and stop using the voided _acquireTimeout—use it instead of hardcoding
any duration and keep the lockAcquireTimeout configuration consistent with this
behavior.
In
`@legacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sql`:
- Around line 72-85: The trigger currently directly references columns that may
not exist at migration time (e.g., NEW.status, NEW.pending_tier, OLD.plan,
NEW.cancel_at_period_end), causing failures; update the trigger logic to first
detect a column's presence (use to_jsonb(NEW) ? 'pending_tier' / to_jsonb(NEW) ?
'status' / to_jsonb(NEW) ? 'cancel_at_period_end' or to_jsonb(OLD) ? 'plan') and
only access or compare NEW.<column>/OLD.<column> when the check returns true,
and guard status comparisons (like status = 'pending_payment') behind the
existence check so the trigger never directly reads a non-existent column.
Ensure all places referencing status, pending_tier, cancel_at_period_end, or
plan (the clauses around TG_OP, the IF NEW/OLD comparisons and event assignments
such as subscription_activated) are updated to use these existence guards before
accessing the fields.
- Around line 33-35: The policy "service_role_sub_log" on table
subscription_activity_log is currently unscoped (FOR ALL USING (true)) which
allows any role with table privileges to read/write; change the policy to
restrict it to the service_role by adding a TO service_role clause and tighten
the USING and WITH CHECK expressions to only allow intended access (e.g., USING
(auth.role() = 'service_role') or other explicit condition) so reads/writes are
limited to the service_role and only permitted rows; update the CREATE POLICY
"service_role_sub_log" definition to include TO service_role and appropriate
USING/ WITH CHECK predicates instead of USING (true).
In
`@legacy/cediwise-dashboard_db_migrations/20260411120000_announcement_target_user.sql`:
- Around line 15-17: The existing constraint
announcement_campaigns_audience_type_check only restricts audience_type values
but allows audience_type = 'single_user' with a NULL target_user_id; update the
table constraints on announcement_campaigns to add a check that when
audience_type = 'single_user' then target_user_id IS NOT NULL (and optionally
enforce that for non-'single_user' values target_user_id IS NULL) so targeted
campaigns always have a target_user_id; modify or add a check constraint
referencing audience_type and target_user_id to enforce this invariant.
In `@legacy/cediwise-dashboard_db_migrations/20260415_create_sms_campaigns.sql`:
- Around line 5-45: Enable Row-Level Security on public.sms_campaigns and
public.sms_recipients and add restrictive policies: enable RLS for both tables
and create policies that only allow SELECT/UPDATE/DELETE/INSERT when the current
user is authorized (e.g., campaign rows where created_by = current_user_id or
the user is in the campaign audience), and for recipients only allow access when
the requester owns the campaign (campaign_id -> sms_campaigns.id) or the
recipient.user_id matches current_user_id; ensure policies explicitly prevent
exposing phone and error_message to unauthorized readers by creating a read
policy that restricts those columns or limits SELECT to authorized roles only,
and add an admin/maintenance policy for system roles if needed.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-02_profiles_budget_personalization_vitals.sql`:
- Around line 25-77: The checks that look up existing constraints by conname
(e.g., profiles_stable_salary_nonneg, profiles_utilities_mode_check,
profiles_needs_pct_check, etc.) must be scoped to the public.profiles table so
they don't collide with same-named constraints on other tables; update each "if
not exists (select 1 from pg_constraint where conname = '<name>')" to also
ensure conrelid = 'public.profiles'::regclass (or equivalently join
pg_class/pg_namespace and filter relname='profiles' and nspname='public') before
deciding to add the constraint to public.profiles.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_adjustments_log_table.sql`:
- Around line 18-21: The check for the constraint only filters by conname and
can falsely detect a same-named constraint on another table; change the
existence check to scope to the target table by adding a conrelid filter (e.g.,
WHERE conname = 'budget_adjustments_log_type_check' AND conrelid =
'public.budget_adjustments_log'::regclass) so the IF NOT EXISTS only skips
creation when that constraint exists on public.budget_adjustments_log; keep the
ALTER TABLE block and constraint name budget_adjustments_log_type_check as-is.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_categories_enhancements.sql`:
- Around line 15-24: The checks that guard adding constraints currently query
pg_constraint by conname only (budget_categories_suggested_limit_check and
budget_categories_no_self_parent) which can match constraints on other tables;
modify both IF NOT EXISTS queries to also filter by conrelid =
'public.budget_categories'::regclass so the lookup is scoped to the
budget_categories table before running the ALTER TABLE ... ADD CONSTRAINT
statements for suggested_limit and parent_id.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_templates_table.sql`:
- Around line 15-16: Add a DB-level constraint to ensure only one template can
be marked as default by creating a unique partial index on the is_default column
for the budget_templates table (i.e., enforce uniqueness where is_default =
true); if templates are scoped per owner/tenant, include that owner/tenant
column in the unique index alongside the is_default predicate. Update the table
definition/migration that currently declares is_default and sort_order so it
adds this unique partial index after the columns are created.
In `@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_debts_table.sql`:
- Around line 24-49: The constraint-existence checks currently query
pg_constraint by conname only and can match constraints on other tables; update
each guard (e.g., checks for 'debts_total_amount_check',
'debts_remaining_amount_check', 'debts_monthly_payment_check',
'debts_interest_rate_check', 'debts_remaining_total_check',
'debts_target_date_check') to also filter by conrelid = 'public.debts'::regclass
so the IF NOT EXISTS queries only consider constraints on the public.debts table
before running the ALTER TABLE ... ADD CONSTRAINT statements.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_spending_patterns_table.sql`:
- Around line 7-9: The current foreign keys on category_id and cycle_id allow
cross-tenant links because they only reference public.budget_categories(id) and
public.budget_cycles(id); change them to composite foreign keys that also
enforce the owning user by referencing (id, user_id) on the target tables (e.g.,
replace references to public.budget_categories(id) and public.budget_cycles(id)
with references to public.budget_categories(id, user_id) and
public.budget_cycles(id, user_id)), ensuring the local columns (category_id,
user_id) and (cycle_id, user_id) are used in the FK; apply the same change for
the other occurrences of user_id/category_id/cycle_id noted in the file (lines
47–49).
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-02-04_user_activity_log_table.sql`:
- Around line 32-34: The RLS policy user_activity_log_self on
public.user_activity_log currently uses "for all" which permits UPDATE and
DELETE; replace it with two explicit policies: one policy allowing SELECT for
rows where auth.uid() = user_id and another allowing INSERT where auth.uid() =
new.user_id (or equivalent insert-expression), and do not create any UPDATE or
DELETE policies so those operations are denied; ensure you drop or remove the
existing user_activity_log_self "for all" policy and create the two new policies
that reference auth.uid() and user_id to enforce append-only immutability.
In
`@legacy/cediwise-mobile-app_supabase_loose/2026-03-02_feedback_and_app_versions.sql`:
- Around line 4-29: Enable row-level security on the newly created tables and
add least-privilege policies in the same migration: after creating
public.feedback and public.app_versions, run ALTER TABLE ... ENABLE ROW LEVEL
SECURITY for each table and add targeted policies that (1) allow INSERT into
public.feedback only from your backend/service role or authenticated users
(prevent anonymous INSERT), (2) restrict SELECT on public.feedback so that PII
(feedback.email) is only visible to an internal/admin role or the service role
while regular users can only read non-PII fields, and (3) restrict UPDATE/DELETE
on public.feedback and all access to public.app_versions to internal/service
roles only; also add explicit GRANTs for only the roles that need access.
Reference the schema objects feedback, app_versions and the feedback.email
column plus the index names (idx_feedback_*) when implementing these ALTER
POLICY and GRANT statements so the RLS and least-privilege controls are applied
in the same migration that creates the tables.
In `@legacy/cediwise-mobile-app_supabase_loose/2026-03-25_budget_engine_mode.sql`:
- Around line 10-12: The current existence check only matches conname and can
mis-detect a constraint with the same name on another table; update the
condition that queries pg_constraint to scope it to the profiles table by
requiring the constraint's conrelid to equal the regclass for public.profiles
(e.g., conrelid = 'public.profiles'::regclass) in addition to conname =
'profiles_budget_engine_mode_check', then only run ALTER TABLE public.profiles
ADD CONSTRAINT profiles_budget_engine_mode_check CHECK (budget_engine_mode IN
(...)) when that scoped check returns no rows.
In
`@legacy/cediwise-mobile-app_supabase_loose/20260407235057_profiles_version.sql`:
- Line 4: The ALTER TABLE statement adds profile_version to the unqualified
table name profiles which can hit the wrong table depending on search_path;
update the migration DDL so the ALTER TABLE targets public.profiles (keep IF NOT
EXISTS and DEFAULT 0), i.e. replace the unqualified table reference in the ALTER
TABLE ... ADD COLUMN statement and also scan this migration for any other
unqualified references to profiles to schema-qualify them as public.profiles.
In
`@legacy/cediwise-mobile-app_supabase_migrations/2026-03-10_budget_transaction_debt_id.sql`:
- Around line 2-3: Add an index for the new foreign-key column to avoid slow
deletes/updates and lookups: after adding budget_transactions.debt_id (the FK
referencing public.debts(id)), create a supporting index on
budget_transactions.debt_id (e.g., name it idx_budget_transactions_debt_id) so
operations on public.debts that reference budget_transactions and queries
filtering by debt_id are efficient.
In
`@legacy/cediwise-mobile-app_supabase_migrations/2026-03-10_debt_source_cycle_id.sql`:
- Around line 2-3: Add an index on the new foreign-key column to avoid table
scans/locks when parent rows in budget_cycles(id) are updated/deleted: create a
btree index on public.debts.source_cycle_id (use a descriptive name like
idx_debts_source_cycle_id) and consider creating it CONCURRENTLY in production
to avoid locking; ensure the migration adds the index after adding the column
and includes IF NOT EXISTS checks consistent with the existing schema
conventions.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260327000000_subscriptions.sql`:
- Around line 23-25: The current policy subscriptions_self on
public.subscriptions uses "FOR ALL" allowing clients to mutate billing fields;
change it to allow only SELECT for authenticated users by replacing the FOR ALL
policy with a FOR SELECT policy (retain using (auth.uid() = user_id)) and
remove/deny client-side FOR INSERT/UPDATE/DELETE checks so clients cannot write
plan/status/paystack IDs; ensure all subscription mutations are performed only
by backend/service-role functions (e.g., the Paystack webhook handler) or a
controlled RPC, and update references in client code (like upgrade.tsx) to stop
performing direct writes to public.subscriptions.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260327000100_profiles_tier.sql`:
- Around line 5-13: After adding the new columns to public.profiles (tier,
trial_ends_at, trial_granted), add an explicit backfill UPDATE in the same
migration that sets profiles.tier and the trial fields from the canonical
subscription source (e.g., your subscriptions/paystack table or materialized
view) so existing paid/trial users are not left as 'free'; specifically, locate
the authoritative subscription table (for example subscriptions or
paystack_subscriptions), join it to public.profiles on the profile identifier,
and run an UPDATE to set tier = 'sme'/'budget'/'free' based on subscription
plan/status and set trial_ends_at and trial_granted from the subscription
metadata, taking care to only update rows where the derived value differs from
the default and to run inside the same migration transaction so application
reads see the correct values immediately.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260327000400_sme_transactions.sql`:
- Around line 14-18: Add CHECK constraints to enforce VAT integrity: ensure
vat_amount is never negative and ensure vat_amount is zero when vat_applicable
is false (or equivalently vat_amount > 0 only when vat_applicable is true).
Locate the table definition that contains the columns vat_applicable and
vat_amount in the migration (the CREATE TABLE or ALTER TABLE for SME
transactions) and add a CHECK (vat_amount >= 0) and a CHECK ((vat_applicable) OR
(vat_amount = 0)) (or a single combined CHECK) to the table; also ensure any
existing data is corrected or validated in the migration before adding the
constraint so it can be applied without failure.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260327000500_seed_sme_categories.sql`:
- Around line 1-29: The timestamped .sql migration currently under the
migrations set only contains commented reference seed data (the Default
INCOME/EXPENSE categories block) and should be removed from the replayable
migration sequence; move this reference out of migrations into a non-timestamped
documentation or seeds folder (e.g., docs/seeds or
seeds/reference_sme_categories), update any README to point to the useSmeLedger
hook as the real insertion point, and replace the removed file in the migrations
list with either a small no-op migration or an explicit migration comment
explaining that category seeding is performed at runtime by useSmeLedger so
auditors won't misread replay results.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260329000100_get_user_count_rpc.sql`:
- Around line 1-6: The SECURITY DEFINER function public.get_user_count currently
allows callers to execute with the definer's privileges and should not be
public; fix by restricting execution and ownership: revoke EXECUTE from PUBLIC
on the get_user_count function, grant EXECUTE only to the intended role(s)
(e.g., your service role), and change the function owner to a dedicated
low-privilege service account instead of a superuser; also ensure the function’s
search_path and any external object access are locked down to avoid unintended
privilege escalation.
- Line 5: The RPC get_user_count is incorrectly capped by a subquery with "LIMIT
100"; replace the limited subquery with an unconditional count of the users
table so the function returns the total number of rows (i.e., remove the
LIMIT/inner select and use a direct count on auth.users in the get_user_count
SQL).
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260402_cash_flow_columns.sql`:
- Around line 23-31: The UPDATE can violate the profiles_cash_flow_income_nonneg
constraint because SUM(amount) from income_sources may be negative; update the
backfill to handle negatives explicitly by transforming sub.total_income before
assigning (e.g., use a CASE or GREATEST to coerce negative totals to 0 or NULL)
or by skipping negative aggregates with an extra WHERE clause; modify the UPDATE
that targets cash_flow_monthly_income (and the subquery over income_sources) to
apply that transformation or filter so no negative value is written.
- Around line 12-18: The existence checks for adding constraints
profiles_cash_flow_balance_nonneg and profiles_cash_flow_income_nonneg currently
query pg_constraint by conname only and may collide with same-named constraints
on other tables; update the IF NOT EXISTS queries to ensure they target the
public.profiles table by checking pg_constraint.conrelid references the
public.profiles OID (use (SELECT oid FROM pg_class WHERE relname='profiles' AND
relnamespace = (SELECT oid FROM pg_namespace WHERE nspname='public')) or
equivalent) and contype = 'c' before running ALTER TABLE public.profiles ADD
CONSTRAINT ... so the checks are scoped to public.profiles and remain
idempotent.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260410140000_vault_initial_balance.sql`:
- Around line 2-4: The migration uses "ADD COLUMN IF NOT EXISTS" which can leave
an existing column without the NOT NULL/default/check invariants; instead make
the change idempotent by: 1) ensure the column exists (ADD COLUMN IF NOT EXISTS
public.profiles.initial_savings_balance numeric), 2) backfill any NULLs (UPDATE
public.profiles SET initial_savings_balance = 0 WHERE initial_savings_balance IS
NULL), 3) set the default and NOT NULL (ALTER TABLE public.profiles ALTER COLUMN
initial_savings_balance SET DEFAULT 0; ALTER TABLE public.profiles ALTER COLUMN
initial_savings_balance SET NOT NULL), and 4) add a named check constraint for
non-negativity if not present (ALTER TABLE public.profiles ADD CONSTRAINT
chk_initial_savings_balance_nonnegative CHECK (initial_savings_balance >= 0));
this sequence guarantees the invariants for both new and pre-existing columns.
In
`@legacy/cediwise-mobile-app_supabase_migrations/20260410140100_vault_deposits.sql`:
- Around line 52-55: The vault_deposits_self row-level policy currently uses
"FOR ALL" which permits UPDATE/DELETE on the immutable public.vault_deposits
ledger; change it to disallow mutations by replacing the single FOR ALL policy
with two targeted policies: one "CREATE POLICY vault_deposits_self_select ON
public.vault_deposits FOR SELECT USING (user_id = auth.uid())" to allow users to
read only their rows, and a separate "CREATE POLICY vault_deposits_self_insert
ON public.vault_deposits FOR INSERT WITH CHECK (user_id = auth.uid())" to allow
inserts only for their own user_id; remove or do not create any UPDATE or DELETE
policy so UPDATE/DELETE remain denied for vault_deposits.
In `@supabase/_stash/99999999999999_fix_schema_drift_if_missing.sql`:
- Line 6: Remove the explicit transaction delimiters "begin;" (and matching
"commit;") from the migration content because Supabase CLI already wraps
migrations in a transaction; keep them only if this SQL is intended as a manual
one-off script in the _stash area—so either delete the "begin;"/ "commit;"
statements from this migration file or document that it is a manual script and
retain them accordingly.
In `@supabase/functions/_shared/plans.ts`:
- Around line 13-49: The two maps PLAN_CARD_INIT and PLAN_MOMO_CHARGE duplicate
the same price/label data; create a single canonical map (e.g., PLANS or
PLAN_DEFS) keyed by plan id containing { plan_code_env?: string, amount_pesewas:
number, label: string } and move the shared metadata there, then derive/export
PLAN_CARD_INIT and PLAN_MOMO_CHARGE from that canonical map (for card use pick
the plan_code_env and for momo use only amount_pesewas/label) so updates happen
in one place; update any references to use the canonical map or the derived
views (retain the same exported names PLAN_CARD_INIT and PLAN_MOMO_CHARGE for
compatibility).
In `@supabase/functions/_shared/proration.ts`:
- Around line 21-23: planKeyToCadence currently defaults unknown plan keys to
"monthly", which can hide invalid input; update the function (planKeyToCadence
in proration.ts) to explicitly detect known cadence tokens (e.g., "monthly" and
"quarterly") and return the corresponding BillingCadence, and for any
other/unknown planKey return null (or throw an Error) so callers can reject the
request; ensure the function signature and callers handle the nullable/exception
behavior consistently.
In `@supabase/functions/billing-upgrade-quote/index.ts`:
- Around line 25-27: The helper fullPricePesewas currently returns 0 when a plan
key has no amount, which yields invalid GH₵0 quotes; update
fullPricePesewas(planKey: string) to detect missing pricing in PLAN_MOMO_CHARGE
and throw an error (or return a sentinel that the caller converts into a 5xx
response) rather than returning 0 so callers (billing-upgrade-quote handler)
surface an operational failure; also adjust the quote handler to catch that
error and respond with a 5xx; optionally, switch the pricing source to the
shared catalog used by checkout instead of PLAN_MOMO_CHARGE to avoid config
drift.
In `@supabase/functions/paystack-initiate/index.ts`:
- Line 86: The endpoint currently logs raw PII in several console.log calls
(e.g., the message string containing userId, checkoutEmail, callbackUrl); update
all console.log usages in this module that interpolate userId, checkoutEmail,
callbackUrl (and any other raw identifiers) to avoid storing PII—either remove
the sensitive fields from the message, replace them with masked values (e.g.,
maskEmail(checkoutEmail) or userId.slice(0,8) + '...'), or log simple
booleans/status flags (e.g., authSucceeded=true, hasCheckoutEmail=true) instead;
ensure changes are applied to every console.log that references the variables
userId, checkoutEmail, or callbackUrl so no raw identifiers or email addresses
are emitted.
In `@supabase/functions/paystack-momo-charge/index.ts`:
- Around line 391-420: The two update calls against the "momo_charge_attempts"
table (the admin.from(...).update(...) chains that currently
.eq("idempotency_key", idempotencyKey)) must also scope by user_id to match the
read keys; add .eq("user_id", userId) to both update query chains so they filter
on both idempotencyKey and userId (use the existing idempotencyKey and userId
variables referenced in this function) to prevent one caller from mutating
another caller's row.
- Around line 223-322: The code treats a pending row with no paystack_reference
as eligible to create a new charge; change the idempotency logic so that if
existing?.status === "pending" you immediately return the pending outcome
(cached: true) regardless of paystack_reference to mark it as in-flight;
additionally, when inserting into admin.from("momo_charge_attempts") with
idempotencyKey fails (insErr), detect unique/constraint race errors and re-query
the same table (using the same idempotencyKey/maybeSingle) to return the
existing pending row instead of returning a 500, so concurrent requests don’t
trigger duplicate external /charge calls.
In `@supabase/functions/paystack-momo-status/index.ts`:
- Around line 76-88: The DB lookup using
admin.from("momo_charge_attempts").select(...).maybeSingle() currently ignores
the returned error and treats failures as a missing reference; update the code
that calls maybeSingle() (the variable destructuring around attempt) to also
capture the error (e.g., { data: attempt, error }) and if error is non-null
return a 5xx Response (include corsHeaders and Content-Type like the 404 branch)
with an explanatory error payload; only proceed to the 404 branch when error is
null and attempt is falsy.
In `@supabase/functions/paystack-webhook/index.ts`:
- Around line 630-698: The code currently starts a grace period for any
charge.failed with a userId; instead gate the update so it only runs for actual
renewal payment failures (not one-off upgrades/prorations). In the charge.failed
branch (around metadataUserId, reconcileMomoByReference, and the subscriptions
update), detect renewal intent — e.g., check a renewal flag in
event.data.metadata (or validate that the failed reference matches the
subscription's expected renewal reference or that sub.status === "active" with a
renewal pending) — and only perform the subscriptions.update (status ->
"grace_period", last_payment_failed_at, grace_period_end) when that renewal
condition is true; otherwise log and skip without modifying the subscription
row. Ensure checks reference metadataUserId, event.data.metadata,
reconcileMomoByReference, and the subscriptions update call.
- Around line 330-349: The handler currently does a read (select ...
maybeSingle) against paystack_applied_charges using the local variable reference
and only later inserts, which allows a race; instead, before applying
subscription side effects in the charge.success handler, perform an atomic claim
by attempting to INSERT the reference into paystack_applied_charges with "on
conflict do nothing" (or the DB client's equivalent) and check the insert
result—if the insert affected 0 rows then skip the rest of the handler; remove
or replace the existing select-based idempotency check (the block using
reference, admin.from("paystack_applied_charges").select(...).maybeSingle()) and
use the insert result to decide whether to proceed, logging errors from the
insert (idemErr) appropriately; apply the same change to the other handlers that
use the same pattern (the blocks around the other reference checks noted at the
other locations).
- Around line 483-489: When handling non-proration charge.success events, don't
anchor renewal windows to now (nowISO/now and periodEnd computed from
addMonths(now,...)); instead detect the existing subscription boundary by
reading current_period_end (and current_period_start) and use that as the
anchor: compute keyCadence/isQuarterly from planKey as before, parse the
existing current_period_end into a Date and if that existing end is in the
future use it as the base to call addMonths(existingEnd, isQuarterly?3:1) and
set current_period_start to the previous end; if no valid future
current_period_end exists fall back to now; apply this same change in both the
first renewal block (where nowISO/periodEnd are set) and the second analogous
block (previously at lines ~552-564) so renewals extend from the subscription’s
renewal boundary rather than webhook receipt time.
In `@supabase/functions/send-announcement/index.ts`:
- Around line 88-95: The current logic builds deviceQuery first and then only
applies .eq("user_id", row.target_user_id) conditionally, so when
row.audience_type === "single_user" but row.target_user_id is null it
unintentionally becomes an unfiltered broadcast; change the flow to validate
row.audience_type and row.target_user_id up front: if row.audience_type ===
"single_user" and row.target_user_id is falsy, fail closed by returning an
error/aborting (e.g., throw or respond with 400) rather than executing the
supabase.from("push_devices") query; otherwise build deviceQuery (the variable
deviceQuery and the .eq calls) only after validation so the .eq("user_id", ...)
is guaranteed to be applied for single_user campaigns.
- Around line 29-64: Before instantiating the privileged client
(SUPABASE_SERVICE_ROLE_KEY / createClient) in the Deno.serve handler, validate
the caller is an admin: extract the Authorization bearer token from req, verify
it (e.g., via a safe public/anon supabase client or JWT verification) and fetch
the user record/claims, then check a clear admin claim/role (e.g.,
app_metadata.role or a custom claim) and return 403 if not an admin; only after
that permit continuing with campaignId processing and creating the service-role
client. Ensure the authorization check runs before any privileged reads/writes
and reference the existing symbols req, campaignId, SUPABASE_SERVICE_ROLE_KEY,
and createClient when adding the checks.
- Around line 190-209: The update currently always sets
announcement_campaigns.status to "sent" even when all deliveries failed; change
the logic in the update call so status is conditional: if success === 0 set
status = "failed", else if failure > 0 set status = "partial" (or
"sent_with_failures"), otherwise set status = "sent"; also ensure error_message
persists when failure > 0 and adjust any downstream Response body to reflect the
chosen status; locate the .from("announcement_campaigns").update(...) call and
modify the status and error_message values accordingly.
- Around line 66-86: The code unconditionally sets campaign.status to "sending",
causing duplicate sends; change the update to claim the campaign atomically by
updating only when its current status is the expected prior state (e.g.,
"pending") and abort if no row was claimed. Replace the existing
supabase.from("announcement_campaigns").update({ status: "sending" }).eq("id",
row.id) call with a conditional update that includes .eq("status","pending") (or
the prior state you expect) and use the returned result/row count (or
.maybeSingle() result) to detect whether the update actually affected a row; if
nothing was updated, return/abort without sending. Ensure you reference the same
campaign/row variables (campaign, row) when performing this conditional claim.
In `@supabase/migrations/20260421191814_remote_schema.sql`:
- Around line 88-109: The SECURITY DEFINER function get_user_learning_summary
allows any caller to pass target_user_id and read another user’s aggregated
data; change it so callers cannot arbitrarily supply target_user_id — either
remove SECURITY DEFINER (use SECURITY INVOKER) or add an explicit authorization
check inside the function that compares target_user_id to auth.uid() or verifies
the caller is an admin before returning results, and ensure execute privileges
are not granted to anon/authenticated; update the function signature/body
accordingly and adjust grants so only authorized roles can call
get_user_learning_summary.
- Around line 65-72: The cleanup function public.cleanup_old_analytics is
defined SECURITY DEFINER and thus can be invoked by anon/auth roles to delete
analytics; fix by removing broad definer privileges and restricting execution to
the service role: either change the function to SECURITY INVOKER or keep
SECURITY DEFINER but immediately revoke EXECUTE FROM public and grant EXECUTE
only to the intended service role (e.g., service_role), ensuring the function
signature cleanup_old_analytics() is updated/altered accordingly so regular
client roles cannot call it.
- Around line 399-409: The migration currently leaves sensitive tables
app_versions, feedback, email_campaigns, email_recipients, sms_campaigns, and
sms_recipients without RLS and then grants ALL to anon/authenticated; fix by
enabling row level security on each table (ALTER TABLE ... ENABLE ROW LEVEL
SECURITY), remove or revoke the broad GRANTs to anon/authenticated, and add
explicit policies that allow only service-role/admin contexts to
SELECT/INSERT/UPDATE/DELETE (e.g., CREATE POLICY "service_role_*" USING
(auth.role() = 'service_role' OR auth.role() = 'admin') and similar FOR ALL or
per-command policies) while providing minimal least-privilege access for any
required authenticated reads via narrow USING/WITH CHECK expressions; apply this
same pattern for app_versions, feedback, email_campaigns, email_recipients,
sms_campaigns, and sms_recipients.
- Around line 78-82: The get_user_count() SQL function is incorrectly capped by
an inner "limit 100", causing counts to never exceed 100; edit the function
definition for "public"."get_user_count" to remove the limiting subquery and
simply return count(*) from auth.users (or an equivalent unbounded aggregate
over auth.users) so it reports the actual total user count, then re-create the
function accordingly.
- Line 2181: Update the permissive policy "service_role_sub_log" on
"subscription_activity_log" to explicitly limit who and which commands it
applies to by adding FOR and TO clauses; change the statement CREATE POLICY
"service_role_sub_log" ON "public"."subscription_activity_log" USING (true) to
include the intended command scope and role (for example: FOR ALL TO
service_role USING (true) or FOR SELECT TO service_role USING (true)) so only
the service_role can exercise that policy; after changing the policy, re-check
GRANTs to anon/authenticated/service_role to ensure they do not reintroduce
public access.
In `@supabase/migrations/20260421193238_remote_schema.sql`:
- Around line 165-176: The view module_completion_stats currently computes
completion_rate using avg(CASE ...) which weights by events; change it to a
user-based ratio by replacing that expression with: the count of DISTINCT
user_id filtered where event_type = 'module_completed' divided by the count of
DISTINCT user_id (i.e., distinct users who engaged the module), casting to
numeric and wrapping the denominator in NULLIF(...,0) to avoid division-by-zero;
keep the rest of the view (module_id, total_users, total_views,
total_quiz_attempts) and use public.literacy_events with its module_id, user_id,
and event_type filters to implement this.
In `@supabase/migrations/20260422171000_subscription_billing_v2.sql`:
- Around line 163-168: The branch assigning event for status transitions
incorrectly maps new_status = 'pending_payment' to 'subscription_activated';
update the logic in the trigger/function that sets the local variable event (the
code using new_status, new_tier and event) so that 'pending_payment' either does
not generate an activity log (skip/return) or sets a distinct event name like
'subscription_pending_payment' instead of 'subscription_activated', and ensure
downstream callers that rely on event are adjusted accordingly.
- Around line 134-149: In the log_subscription_change trigger function you must
not dereference OLD on INSERT; move assignments that use OLD (old_tier :=
coalesce(OLD.plan, 'free') and old_status := coalesce(OLD.status, 'active'))
into the branch that handles UPDATE/DELETE (i.e., when TG_OP = 'UPDATE' or
'DELETE'), and similarly ensure assignments using NEW (new_tier :=
coalesce(NEW.plan, 'free') and new_status := coalesce(NEW.status, 'active'))
occur only in the INSERT/UPDATE branch (when NEW is present); update the
conditional branches in log_subscription_change to set old_tier/old_status and
new_tier/new_status in the appropriate TG_OP-specific blocks so no NULL record
is dereferenced.
In `@supabase/migrations/20260423170500_feedback_rating_not_null_restore.sql`:
- Around line 3-15: The migration only normalizes NULL ratings but not
out-of-range values, which will cause ADD CONSTRAINT feedback_rating_check to
fail; update the public.feedback rows where rating IS NULL OR rating < 1 OR
rating > 5 to a valid default (e.g., 3) before dropping/adding the constraint
and setting rating NOT NULL, so change the normalization step that targets
rating to include all invalid values for the rating column in table
public.feedback prior to adding the CHECK constraint.
In
`@supabase/migrations/20260424120000_billing_webhook_dedupe_realtime_auto_downgrade.sql`:
- Around line 104-105: The trigger currently sets event := 'auto_downgraded' in
the elsif branch (elsif old_status = 'grace_period' and new_status = 'expired'
and new_tier = 'free') but the subscription_activity_log_event_type_check
expansion that allows that value doesn't exist yet; change the logic in that
trigger (the block that assigns to event) so it only assigns 'auto_downgraded'
when the database actually accepts that enum/value (e.g. guard with an existence
check against pg_enum or the subscription_activity_log_event_type regtype), and
otherwise set a safe fallback (NULL or an existing event value) to avoid
inserting a disallowed value and causing the trigger insert to fail.
In `@supabase/migrations/20260424160000_billing_cycle_proration.sql`:
- Around line 91-97: The trigger currently dereferences OLD (old_tier :=
coalesce(OLD.plan, 'free'); old_status := coalesce(OLD.status, 'active')) before
checking TG_OP, which causes "record OLD is not assigned yet" on INSERT; move
those OLD-based assignments into the UPDATE branch (after the TG_OP check) and
in the INSERT branch set old_tier and old_status to NULL or derive from NEW
(e.g., new_tier := coalesce(NEW.plan,'free')) so only UPDATE uses OLD; update
any logic that references old_tier/old_status to account for NULL in INSERT.
In `@supabase/migrations/README.md`:
- Line 5: The README currently hard-codes the Supabase project ref in the
command string "supabase link --project-ref etilowirjbuyknsfdtpt"; replace that
literal project ref with a placeholder or env-var token (e.g. {PROJECT_REF} or
$SUPABASE_PROJECT_REF) and update the example line so contributors are
instructed to run "supabase login" then "supabase link --project-ref
<PROJECT_REF>" (or substitute an env var) to avoid linking to the wrong project.
In `@supabase/README.md`:
- Around line 23-31: The fenced code block in README.md triggers markdownlint
MD040 because it lacks a language; update the block delimiter from ``` to
```text so the example layout remains intact and the linter warning is
resolved—edit the README.md fenced block that lists the supabase/ directory
entries (the block starting with "supabase/" and ending with "docs/") to use
```text.
In `@supabase/schema.generated.sql`:
- Around line 1-3: The generated schema snapshot is out of date: it still
contains the pre-v2 "subscriptions" table shape and the old
"subscription_activity_log" enum values and is missing the new billing tables;
regenerate the SQL dump after applying the billing v2 migrations so the file
reflects the current schema (run the same command used to create it: "supabase
db dump --linked --schema public -f supabase/schema.generated.sql" or your
project's canonical dump step) and commit the updated
supabase/schema.generated.sql so "subscriptions", "subscription_activity_log"
and the new billing tables appear correctly.
In `@supabase/seeds/seed_budget_templates.sql`:
- Line 6: Replace the unsafe "TRUNCATE ... CASCADE" statement on the budget
templates seed with a non-destructive alternative: remove the CASCADE and either
use "TRUNCATE public.budget_templates" only when you're certain dependent rows
are safe, or better, use a targeted "DELETE FROM public.budget_templates"
(optionally wrapped in a transaction) to avoid deleting dependent data; update
the SQL that contains the "truncate table public.budget_templates cascade;"
statement accordingly and add a comment clarifying when a full truncate is
permitted.
---
Outside diff comments:
In `@supabase/.gitignore`:
- Around line 5-9: The .gitignore currently only ignores .env.local, .env.keys
and .env.*.local leaving supabase/.env and other non-local env files trackable;
update supabase/.gitignore to also ignore the base environment files by adding
entries for .env and .env.* (while preserving any example files like
.env.example), so that .env and any non-local variants are not committed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5c6648d4-bfe4-4538-8dbf-0488b2e2aacb
📒 Files selected for processing (85)
.gitignoreREADME.mdbackups/README.mdcediwise-mobile-app/README.mdcediwise-mobile-app/docs/plans/20260420203916_subscription-billing-system-redesign.mdcediwise-mobile-app/docs/plans/20260421000000_subscription-billing-system-redesign-v2.mdcediwise-mobile-app/utils/supabase.tslegacy/INVENTORY.mdlegacy/README.mdlegacy/cediwise-dashboard_db_migrations/2026-03-02_email_campaigns.sqllegacy/cediwise-dashboard_db_migrations/2026-03-03_email_rich_body_join_beta.sqllegacy/cediwise-dashboard_db_migrations/2026-03-16_add_new_email_templates.sqllegacy/cediwise-dashboard_db_migrations/2026-03-19_add_user_onboarding_state.sqllegacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sqllegacy/cediwise-dashboard_db_migrations/20260411120000_announcement_target_user.sqllegacy/cediwise-dashboard_db_migrations/20260415_create_sms_campaigns.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-02_profiles_budget_personalization_vitals.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-03_budget_utilities_under_needs.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_adjustments_log_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_categories_enhancements.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_cycles_enhancements.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_budget_templates_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_debts_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_profiles_enhancements.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_recurring_expenses_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_spending_patterns_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-04_user_activity_log_table.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-16_flm_tables.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-17_flm_analytics.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-18_profiles_debt_obligations.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-22_admin_users.sqllegacy/cediwise-mobile-app_supabase_loose/2026-02-22_lessons_content.sqllegacy/cediwise-mobile-app_supabase_loose/2026-03-01_push_notifications.sqllegacy/cediwise-mobile-app_supabase_loose/2026-03-02_feedback_and_app_versions.sqllegacy/cediwise-mobile-app_supabase_loose/2026-03-25_budget_engine_mode.sqllegacy/cediwise-mobile-app_supabase_loose/20260407235057_profiles_version.sqllegacy/cediwise-mobile-app_supabase_migrations/2026-03-08_budget_category_icon.sqllegacy/cediwise-mobile-app_supabase_migrations/2026-03-10_budget_transaction_debt_id.sqllegacy/cediwise-mobile-app_supabase_migrations/2026-03-10_debt_source_cycle_id.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000000_subscriptions.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000100_profiles_tier.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000200_sme_profiles.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000300_sme_categories.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000400_sme_transactions.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000500_seed_sme_categories.sqllegacy/cediwise-mobile-app_supabase_migrations/20260327000600_sme_categories_updated_at.sqllegacy/cediwise-mobile-app_supabase_migrations/20260329000100_get_user_count_rpc.sqllegacy/cediwise-mobile-app_supabase_migrations/20260329000200_fix_debts_cycle_fk.sqllegacy/cediwise-mobile-app_supabase_migrations/20260329000300_subscription_refactor.sqllegacy/cediwise-mobile-app_supabase_migrations/20260402_cash_flow_columns.sqllegacy/cediwise-mobile-app_supabase_migrations/20260408023500_app_versions_release_notes_requires_update.sqllegacy/cediwise-mobile-app_supabase_migrations/20260410120000_recurring_expenses_auto_allocate.sqllegacy/cediwise-mobile-app_supabase_migrations/20260410140000_vault_initial_balance.sqllegacy/cediwise-mobile-app_supabase_migrations/20260410140100_vault_deposits.sqllegacy/cediwise-mobile-app_supabase_schema_sql.sqllegacy/cediwise-web-official_db_migrations/2026-03-02_feedback_and_app_versions.sqlsupabase/.gitignoresupabase/README.mdsupabase/_stash/99999999999999_fix_schema_drift_if_missing.sqlsupabase/config.tomlsupabase/functions/_shared/customerDisplayName.tssupabase/functions/_shared/plans.tssupabase/functions/_shared/proration.tssupabase/functions/billing-schedule-cadence/index.tssupabase/functions/billing-upgrade-quote/index.tssupabase/functions/daily-cash-flow-check/index.tssupabase/functions/delete-account/index.tssupabase/functions/paystack-initiate/index.tssupabase/functions/paystack-momo-charge/index.tssupabase/functions/paystack-momo-status/index.tssupabase/functions/paystack-webhook/index.tssupabase/functions/send-announcement/index.tssupabase/functions/subscription-janitor/index.tssupabase/migrations/20260421191814_remote_schema.sqlsupabase/migrations/20260421193238_remote_schema.sqlsupabase/migrations/20260422162200_feedback_mobile_and_announcement_reads.sqlsupabase/migrations/20260422171000_subscription_billing_v2.sqlsupabase/migrations/20260423170500_feedback_rating_not_null_restore.sqlsupabase/migrations/20260424120000_billing_webhook_dedupe_realtime_auto_downgrade.sqlsupabase/migrations/20260424160000_billing_cycle_proration.sqlsupabase/migrations/99999999999999_fix_schema_drift_if_missing.sqlsupabase/migrations/README.mdsupabase/schema.generated.sqlsupabase/seeds/seed_budget_templates.sqlsupabase/seeds/seed_flm_lessons.sql
| backups/ | ||
| !backups/README.md |
There was a problem hiding this comment.
backups/README.md re-include won’t work with current parent ignore rule.
Because backups/ is ignored as a directory, Line 29 won’t reliably unignore README.md.
Suggested fix
-backups/
-!backups/README.md
+backups/*
+!backups/README.md📝 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.
| backups/ | |
| !backups/README.md | |
| backups/* | |
| !backups/README.md |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore around lines 28 - 29, The current .gitignore entry "backups/"
prevents unignoring a file inside that directory; replace the directory-wide
ignore with a pattern that ignores files but allows exceptions: remove or change
"backups/" to "backups/*" and keep or add the exception "!backups/README.md"
(and if needed add "!backups/" before the file exception) so the README can be
re-included; target the "backups/" and "backups/README.md" entries when making
this change.
| #### MoMo Users | ||
| ``` | ||
| Subscribe → MoMo selected by default → Manual pay each cycle | ||
| ↓ | ||
| Reminders sent: 5, 3, 1 days before billing date | ||
| ↓ | ||
| Billing date arrives → Reminder sent | ||
| ↓ | ||
| Grace period: 5 days after billing date | ||
| ↓ | ||
| Day 1, 3, 5: Overdue reminders | ||
| ↓ | ||
| Grace period ends → User taps "Pay Now" → Pays → Access restored | ||
| ``` | ||
|
|
||
| #### Card Users | ||
| ``` |
There was a problem hiding this comment.
Clean up the markdownlint violations in this retained doc.
This file still triggers repeated MD022/MD031/MD040/MD058/MD047 warnings for missing blank lines, language-less fences, table spacing, and the missing trailing newline. Even for a historical spec, keeping it lint-clean avoids noisy CI/editor diagnostics.
Also applies to: 171-171, 247-269, 306-342, 386-434, 451-451
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 61-61: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 62-62: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 76-76: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 77-77: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| const nativeAuthLock = async <R>( | ||
| _name: string, | ||
| _acquireTimeout: number, | ||
| fn: () => Promise<R>, | ||
| ): Promise<R> => { | ||
| void _name; | ||
| void _acquireTimeout; | ||
| const pending = nativeAuthMutex.then(() => fn()); | ||
| nativeAuthMutex = pending.then( | ||
| () => undefined, | ||
| () => undefined, | ||
| ); | ||
| return pending; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For @supabase/supabase-jsv2.93.2 / GoTrue lock API: does a customlock(name, acquireTimeout, fn)implementation need to enforceacquireTimeout, and what are the expected failure semantics when lock acquisition exceeds timeout?
💡 Result:
Yes, a custom lock(name: string, acquireTimeout: number, fn: => Promise) implementation must enforce the acquireTimeout parameter. If lock acquisition exceeds the timeout, it should reject the returned Promise with an error where error.isAcquireTimeout === true. The expected failure semantics are: - acquireTimeout < 0: Wait indefinitely (not recommended, can cause deadlocks). - acquireTimeout === 0: Throw error immediately if lock unavailable (no waiting). - acquireTimeout > 0: Wait up to acquireTimeout ms; reject with timeout error if exceeded. This contract is defined in the source code of built-in implementations (navigatorLock and processLock) in @supabase/auth-js/src/lib/locks.ts, which are used by GoTrueClient._acquireLock. Custom locks are passed the lockAcquireTimeout (default 10000ms) from GoTrueClientOptions and must respect it to prevent hangs/deadlocks. Example no-op lock (ignores timeout, runs fn immediately): const noOpLock = async (name: string, acquireTimeout: number, fn: => Promise) => await fn However, this skips synchronization and risks races. Example timeout-respecting lock: const myLock = async (name: string, acquireTimeout: number, fn: => Promise) => { const controller = new AbortController if (acquireTimeout > 0) { setTimeout( => controller.abort, acquireTimeout) } // Use controller.signal with your lock mechanism // Reject with { isAcquireTimeout: true } on abort return await fn } Official docs recommend checking navigator.locks availability before using navigatorLock as custom lock; use processLock for non-browser/single-process envs like React Native.
Citations:
- 1: https://cdn.jsdelivr.net/npm/@supabase/auth-js@2.100.1/src/lib/locks.ts
- 2: https://unpkg.com/browse/@supabase/gotrue-js@2.65.1/src/lib/locks.ts
- 3: https://github.com/supabase/supabase-js/blob/bd024171/packages/core/auth-js/src/lib/locks.ts
- 4: fix(auth): add configurable lock acquisition timeout to prevent deadlocks supabase/supabase-js#1962
- 5: supabase/supabase-js@bfa55bb
- 6: fix(auth): replace navigatorLock with processLock as default browser lock supabase/supabase-js#2235
🏁 Script executed:
cd cediwise-mobile-app && find . -name "supabase.ts" -type fRepository: teebhagg/CediWise
Length of output: 81
🏁 Script executed:
cd cediwise-mobile-app && git ls-files | grep -E "(supabase|lock)" | head -20Repository: teebhagg/CediWise
Length of output: 97
🏁 Script executed:
cd cediwise-mobile-app && rg "nativeAuthLock|lockAcquireTimeout" -B 2 -A 2Repository: teebhagg/CediWise
Length of output: 742
🏁 Script executed:
cd cediwise-mobile-app && cat -n utils/supabase.tsRepository: teebhagg/CediWise
Length of output: 2586
nativeAuthLock violates the Supabase lock contract by ignoring acquireTimeout
The custom lock implementation (lines 23-35) discards the _acquireTimeout parameter while the Supabase client is configured with lockAcquireTimeout: 60_000 (line 55). Per the Supabase/GoTrue lock API contract, custom locks must enforce this timeout—rejecting with error.isAcquireTimeout === true if acquisition exceeds the timeout.
The code comment (lines 19-20) acknowledges acquireTimeout is "part of the lock API for compatibility" but intentionally ignores it, claiming this "matches the common 'always eventually serializes' mutex pattern." This violates the documented contract and creates a real risk: if any auth operation stalls in the queue, all subsequent auth flows on iOS/Android will block indefinitely.
Options:
- Implement timeout enforcement with rejection semantics matching the contract.
- Switch to the built-in
processLock(if viable for your concurrency model). - If intentional, remove the
lockAcquireTimeoutconfig and clarify the tradeoff in comments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-mobile-app/utils/supabase.ts` around lines 23 - 35, nativeAuthLock
currently ignores the acquireTimeout parameter and can block indefinitely;
update nativeAuthLock to honor the _acquireTimeout by racing the queued
acquisition (nativeAuthMutex.then(() => fn())) against a timeout Promise that
rejects when elapsed with an Error object that has error.isAcquireTimeout ===
true, ensure nativeAuthMutex is still advanced/cleared on both resolve and
reject (so later callers aren't permanently stuck), and stop using the voided
_acquireTimeout—use it instead of hardcoding any duration and keep the
lockAcquireTimeout configuration consistent with this behavior.
| DROP POLICY IF EXISTS "service_role_sub_log" ON public.subscription_activity_log; | ||
| CREATE POLICY "service_role_sub_log" ON public.subscription_activity_log | ||
| FOR ALL USING (true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the migration file
find . -type f -name "*subscription_activity_log*" -o -name "*20260330*"Repository: teebhagg/CediWise
Length of output: 148
🏁 Script executed:
# Search for the migration file more broadly
find . -type f -path "*/legacy/*migrations*.sql" | head -20Repository: teebhagg/CediWise
Length of output: 1750
🏁 Script executed:
# Look at the directory structure for migrations
fd -t f "\.sql$" legacy/ 2>/dev/null | head -20Repository: teebhagg/CediWise
Length of output: 1667
🏁 Script executed:
# Search for RLS policies in the codebase to understand patterns
rg -i "CREATE POLICY" --type sql -A 3 -B 1Repository: teebhagg/CediWise
Length of output: 50373
🏁 Script executed:
# Search for role definitions
rg -i "CREATE ROLE|TO service_role|TO postgres" --type sqlRepository: teebhagg/CediWise
Length of output: 638
🏁 Script executed:
cat -n ./legacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sqlRepository: teebhagg/CediWise
Length of output: 6921
🏁 Script executed:
# Check for role definitions in the codebase
rg -i "create role|alter role" --type sql | head -20Repository: teebhagg/CediWise
Length of output: 43
🏁 Script executed:
# Look for how service_role is typically used with policies
rg -i "TO service_role" --type sql -B 2 -A 2 | head -50Repository: teebhagg/CediWise
Length of output: 2430
🏁 Script executed:
# Look for any grants or other policies on subscription_activity_log in newer migrations
rg "subscription_activity_log" --type sql -B 3 -A 3Repository: teebhagg/CediWise
Length of output: 19104
🏁 Script executed:
# Check PostgreSQL RLS documentation understanding: search for comments explaining policy scoping in the codebase
rg -i "policy.*to (authenticated|service_role|postgres)" --type sql -B 1 -A 1 | head -40Repository: teebhagg/CediWise
Length of output: 43
🌐 Web query:
PostgreSQL Row Level Security TO clause policy scope behavior
💡 Result:
In PostgreSQL Row Level Security (RLS), the TO clause in CREATE POLICY specifies the role(s) the policy applies to. If omitted or set to PUBLIC, it applies to all roles. Multiple roles can be listed, separated by commas. A policy applies to a querying user if the user is a member of (inherits from) any role named in the TO clause, following standard PostgreSQL role membership and inheritance rules. For example, CREATE POLICY p1 ON t TO role1, role2 applies to users who are role1 or role2 or members thereof. Policies are evaluated in the security context of the querying user (current_user for permission checks within policy expressions). Policy expressions run with the privileges of the user executing the query. If no applicable permissive policies grant access (and restrictive policies pass), access is denied (default-deny when RLS enabled). Superusers and BYPASSRLS roles bypass RLS. Table owners bypass unless FORCE ROW LEVEL SECURITY is set. Example from docs: CREATE POLICY account_managers ON accounts TO managers USING (manager = current_user); applies only to 'managers' role members, allowing access to rows where manager matches their current_user.
Citations:
- 1: https://www.postgresql.org/docs/16/ddl-rowsecurity.html
- 2: https://www.postgresql.org/docs/current/ddl-rowsecurity.html
- 3: http://www.postgresql.org/docs/15/ddl-rowsecurity.html
- 4: https://www.postgresql.org/docs/17/sql-createpolicy.html
- 5: https://www.postgresql.org/docs/current/sql-createpolicy.html
- 6: https://www.postgresql.org/docs/current/static/ddl-rowsecurity.html
Scope this policy to service_role.
Without a TO service_role clause, this policy applies to all roles with table privileges. USING (true) allows any authenticated user, anon user, or service_role to read and write all rows unconditionally. This exposes subscription history (tier changes, payment failures, cancellations) across all users and permits arbitrary modifications to the log.
Suggested policy hardening
CREATE POLICY "service_role_sub_log" ON public.subscription_activity_log
- FOR ALL USING (true);
+ FOR ALL
+ TO service_role
+ USING (true)
+ WITH CHECK (true);📝 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.
| DROP POLICY IF EXISTS "service_role_sub_log" ON public.subscription_activity_log; | |
| CREATE POLICY "service_role_sub_log" ON public.subscription_activity_log | |
| FOR ALL USING (true); | |
| DROP POLICY IF EXISTS "service_role_sub_log" ON public.subscription_activity_log; | |
| CREATE POLICY "service_role_sub_log" ON public.subscription_activity_log | |
| FOR ALL | |
| TO service_role | |
| USING (true) | |
| WITH CHECK (true); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@legacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sql`
around lines 33 - 35, The policy "service_role_sub_log" on table
subscription_activity_log is currently unscoped (FOR ALL USING (true)) which
allows any role with table privileges to read/write; change the policy to
restrict it to the service_role by adding a TO service_role clause and tighten
the USING and WITH CHECK expressions to only allow intended access (e.g., USING
(auth.role() = 'service_role') or other explicit condition) so reads/writes are
limited to the service_role and only permitted rows; update the CREATE POLICY
"service_role_sub_log" definition to include TO service_role and appropriate
USING/ WITH CHECK predicates instead of USING (true).
| ELSIF new_status = 'pending_payment' THEN | ||
| event := 'subscription_activated'; | ||
| ELSIF new_status = 'active' AND new_tier != 'free' THEN | ||
| event := 'subscription_activated'; | ||
| ELSE | ||
| event := 'subscription_activated'; | ||
| END IF; | ||
|
|
||
| ELSIF TG_OP = 'UPDATE' THEN | ||
| -- Skip if nothing tier-relevant changed | ||
| IF OLD.plan IS NOT DISTINCT FROM NEW.plan AND | ||
| OLD.status IS NOT DISTINCT FROM NEW.status AND | ||
| OLD.pending_tier IS NOT DISTINCT FROM NEW.pending_tier THEN | ||
| RETURN NEW; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*20260330000000_subscription_activity_log.sql" | head -20Repository: teebhagg/CediWise
Length of output: 148
🏁 Script executed:
# List files in the legacy migrations directory to understand the structure
fd . legacy/cediwise-dashboard_db_migrations/ -type f -name "*.sql" | head -20Repository: teebhagg/CediWise
Length of output: 232
🏁 Script executed:
cat -n ./legacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sqlRepository: teebhagg/CediWise
Length of output: 6921
🏁 Script executed:
# Search for subscriptions table definition in migrations directory
fd . legacy/cediwise-dashboard_db_migrations/ -name "*.sql" -exec grep -l "CREATE TABLE.*subscriptions" {} \;Repository: teebhagg/CediWise
Length of output: 292
🏁 Script executed:
# Search for subscriptions table definition
fd . legacy/cediwise-dashboard_db_migrations/ -name "*.sql" -exec grep -l "CREATE TABLE.*subscriptions" {} \;Repository: teebhagg/CediWise
Length of output: 292
🏁 Script executed:
# Check all migrations to find subscriptions table
rg "CREATE TABLE.*subscriptions" legacy/cediwise-dashboard_db_migrations/ -A 30Repository: teebhagg/CediWise
Length of output: 43
🏁 Script executed:
# List all SQL migration files
ls -la legacy/cediwise-dashboard_db_migrations/ | grep "\.sql"Repository: teebhagg/CediWise
Length of output: 679
🏁 Script executed:
# Search for "subscriptions" table definition more broadly
rg "subscriptions" legacy/cediwise-dashboard_db_migrations/ | grep -i "create\|alter\|column" | head -40Repository: teebhagg/CediWise
Length of output: 43
🏁 Script executed:
# Search for subscriptions table in the entire repo
rg "CREATE TABLE.*subscriptions" . | head -20Repository: teebhagg/CediWise
Length of output: 256
🏁 Script executed:
# Also check supabase migrations
find . -path "*/migrations*" -name "*.sql" -type f | head -20Repository: teebhagg/CediWise
Length of output: 612
🏁 Script executed:
# Get the subscriptions table definition from the schema
rg "CREATE TABLE.*subscriptions" ./supabase/schema.generated.sql -A 50Repository: teebhagg/CediWise
Length of output: 2715
🏁 Script executed:
# Also check the migration
rg "CREATE TABLE.*subscriptions" ./supabase/migrations/20260421191814_remote_schema.sql -A 50Repository: teebhagg/CediWise
Length of output: 2664
🏁 Script executed:
# Check if subscriptions is defined in the legacy migrations directory
cat legacy/cediwise-dashboard_db_migrations/*.sql | rg "subscriptions"Repository: teebhagg/CediWise
Length of output: 285
🏁 Script executed:
# Check the exact status values allowed in the actual schema
rg "subscriptions_status_check" ./supabase/Repository: teebhagg/CediWise
Length of output: 1065
🏁 Script executed:
# Check the exact dates and order of migrations
ls -la legacy/cediwise-dashboard_db_migrations/ supabase/migrations/ | grep -E "\.sql$"Repository: teebhagg/CediWise
Length of output: 1454
🏁 Script executed:
# Verify the schema drift file to understand what changed
cat ./supabase/migrations/99999999999999_fix_schema_drift_if_missing.sql | head -100Repository: teebhagg/CediWise
Length of output: 1452
The trigger assumes schema fields that don't exist in the subscriptions table definition at migration time.
Lines 72, 121, 125 reference status = 'pending_payment', which is not in the initial subscriptions constraint (April 21 migration). Lines 84, 101, 105, 149 reference pending_tier column, and line 148 references cancel_at_period_end column. While a corrective migration (99999999999999_fix_schema_drift_if_missing.sql) later adds these fields, the trigger runs first and will fail trying to access non-existent columns or check invalid status values before the drift correction executes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@legacy/cediwise-dashboard_db_migrations/20260330000000_subscription_activity_log.sql`
around lines 72 - 85, The trigger currently directly references columns that may
not exist at migration time (e.g., NEW.status, NEW.pending_tier, OLD.plan,
NEW.cancel_at_period_end), causing failures; update the trigger logic to first
detect a column's presence (use to_jsonb(NEW) ? 'pending_tier' / to_jsonb(NEW) ?
'status' / to_jsonb(NEW) ? 'cancel_at_period_end' or to_jsonb(OLD) ? 'plan') and
only access or compare NEW.<column>/OLD.<column> when the check returns true,
and guard status comparisons (like status = 'pending_payment') behind the
existence check so the trigger never directly reads a non-existent column.
Ensure all places referencing status, pending_tier, cancel_at_period_end, or
plan (the clauses around TG_OP, the IF NEW/OLD comparisons and event assignments
such as subscription_activated) are updated to use these existence guards before
accessing the fields.
| UPDATE public.feedback | ||
| SET rating = 3 | ||
| WHERE rating IS NULL; | ||
|
|
||
| ALTER TABLE public.feedback | ||
| DROP CONSTRAINT IF EXISTS feedback_rating_check; | ||
|
|
||
| ALTER TABLE public.feedback | ||
| ADD CONSTRAINT feedback_rating_check | ||
| CHECK (rating >= 1 AND rating <= 5); | ||
|
|
||
| ALTER TABLE public.feedback | ||
| ALTER COLUMN rating SET NOT NULL; |
There was a problem hiding this comment.
Normalize all invalid ratings before adding the range check.
Only NULLs are fixed currently; existing out-of-range values can still break ADD CONSTRAINT.
Suggested fix
-UPDATE public.feedback
-SET rating = 3
-WHERE rating IS NULL;
+UPDATE public.feedback
+SET rating = 3
+WHERE rating IS NULL
+ OR rating < 1
+ OR rating > 5;📝 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.
| UPDATE public.feedback | |
| SET rating = 3 | |
| WHERE rating IS NULL; | |
| ALTER TABLE public.feedback | |
| DROP CONSTRAINT IF EXISTS feedback_rating_check; | |
| ALTER TABLE public.feedback | |
| ADD CONSTRAINT feedback_rating_check | |
| CHECK (rating >= 1 AND rating <= 5); | |
| ALTER TABLE public.feedback | |
| ALTER COLUMN rating SET NOT NULL; | |
| UPDATE public.feedback | |
| SET rating = 3 | |
| WHERE rating IS NULL | |
| OR rating < 1 | |
| OR rating > 5; | |
| ALTER TABLE public.feedback | |
| DROP CONSTRAINT IF EXISTS feedback_rating_check; | |
| ALTER TABLE public.feedback | |
| ADD CONSTRAINT feedback_rating_check | |
| CHECK (rating >= 1 AND rating <= 5); | |
| ALTER TABLE public.feedback | |
| ALTER COLUMN rating SET NOT NULL; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260423170500_feedback_rating_not_null_restore.sql`
around lines 3 - 15, The migration only normalizes NULL ratings but not
out-of-range values, which will cause ADD CONSTRAINT feedback_rating_check to
fail; update the public.feedback rows where rating IS NULL OR rating < 1 OR
rating > 5 to a valid default (e.g., 3) before dropping/adding the constraint
and setting rating NOT NULL, so change the normalization step that targets
rating to include all invalid values for the rating column in table
public.feedback prior to adding the CHECK constraint.
| elsif old_status = 'grace_period' and new_status = 'expired' and new_tier = 'free' then | ||
| event := 'auto_downgraded'; |
There was a problem hiding this comment.
Don't emit auto_downgraded before the log constraint accepts it.
This migration starts writing auto_downgraded, but the subscription_activity_log_event_type_check expansion that allows that value does not arrive until 20260424160000_billing_cycle_proration.sql. If a grace-period expiry happens in between those two migrations, the trigger insert will fail and roll back the subscription update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@supabase/migrations/20260424120000_billing_webhook_dedupe_realtime_auto_downgrade.sql`
around lines 104 - 105, The trigger currently sets event := 'auto_downgraded' in
the elsif branch (elsif old_status = 'grace_period' and new_status = 'expired'
and new_tier = 'free') but the subscription_activity_log_event_type_check
expansion that allows that value doesn't exist yet; change the logic in that
trigger (the block that assigns to event) so it only assigns 'auto_downgraded'
when the database actually accepts that enum/value (e.g. guard with an existence
check against pg_enum or the subscription_activity_log_event_type regtype), and
otherwise set a safe fallback (NULL or an existing event value) to avoid
inserting a disallowed value and causing the trigger insert to fail.
| ``` | ||
| supabase/ | ||
| config.toml # CLI config (from supabase init) | ||
| migrations/ # ONLY source of DDL for production | ||
| functions/ # Edge Functions (deploy: supabase functions deploy <name>) | ||
| seeds/ # Optional seed data for local db reset | ||
| schema.generated.sql # Read-only DDL snapshot (regenerate after changes) | ||
| docs/ # Runbooks + non-schema inventory template | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced block.
This example trips markdownlint MD040. Using ```text keeps the layout example intact and clears the warning.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/README.md` around lines 23 - 31, The fenced code block in README.md
triggers markdownlint MD040 because it lacks a language; update the block
delimiter from ``` to ```text so the example layout remains intact and the
linter warning is resolved—edit the README.md fenced block that lists the
supabase/ directory entries (the block starting with "supabase/" and ending with
"docs/") to use ```text.
| -- Run after budget_templates table migration | ||
|
|
||
| -- Clear existing templates (for re-running) | ||
| truncate table public.budget_templates cascade; |
There was a problem hiding this comment.
TRUNCATE ... CASCADE is unsafe for shared/staging environments.
Line 6 can delete dependent application data beyond templates. This is a high-risk pattern for any non-ephemeral DB run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/seeds/seed_budget_templates.sql` at line 6, Replace the unsafe
"TRUNCATE ... CASCADE" statement on the budget templates seed with a
non-destructive alternative: remove the CASCADE and either use "TRUNCATE
public.budget_templates" only when you're certain dependent rows are safe, or
better, use a targeted "DELETE FROM public.budget_templates" (optionally wrapped
in a transaction) to avoid deleting dependent data; update the SQL that contains
the "truncate table public.budget_templates cascade;" statement accordingly and
add a comment clarifying when a full truncate is permitted.
| function fullPricePesewas(planKey: string): number { | ||
| return PLAN_MOMO_CHARGE[planKey]?.amount_pesewas ?? 0; | ||
| } |
There was a problem hiding this comment.
Fail closed when a plan has no configured amount.
fullPricePesewas() silently returns 0, and the success paths below will surface that as a valid quote. In a config drift scenario this becomes a GH₵0 upgrade quote instead of an operational error. Return a 5xx for missing plan pricing, and ideally source quote amounts from the same shared catalog used by checkout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/billing-upgrade-quote/index.ts` around lines 25 - 27, The
helper fullPricePesewas currently returns 0 when a plan key has no amount, which
yields invalid GH₵0 quotes; update fullPricePesewas(planKey: string) to detect
missing pricing in PLAN_MOMO_CHARGE and throw an error (or return a sentinel
that the caller converts into a 5xx response) rather than returning 0 so callers
(billing-upgrade-quote handler) surface an operational failure; also adjust the
quote handler to catch that error and respond with a 5xx; optionally, switch the
pricing source to the shared catalog used by checkout instead of
PLAN_MOMO_CHARGE to avoid config drift.
| } | ||
|
|
||
| const userId = userData.user.id; | ||
| console.log(`[paystack-initiate] Auth OK — userId: ${userId}`); |
There was a problem hiding this comment.
Remove raw user/email logging from this billing endpoint.
These log lines emit the full userId, callback URL, and checkout email on a sensitive payment path. That creates unnecessary PII retention in logs; prefer booleans or masked values instead of raw identifiers and email addresses.
Also applies to: 95-95, 130-142, 214-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/paystack-initiate/index.ts` at line 86, The endpoint
currently logs raw PII in several console.log calls (e.g., the message string
containing userId, checkoutEmail, callbackUrl); update all console.log usages in
this module that interpolate userId, checkoutEmail, callbackUrl (and any other
raw identifiers) to avoid storing PII—either remove the sensitive fields from
the message, replace them with masked values (e.g., maskEmail(checkoutEmail) or
userId.slice(0,8) + '...'), or log simple booleans/status flags (e.g.,
authSucceeded=true, hasCheckoutEmail=true) instead; ensure changes are applied
to every console.log that references the variables userId, checkoutEmail, or
callbackUrl so no raw identifiers or email addresses are emitted.
| const { data: existing } = await admin | ||
| .from("momo_charge_attempts") | ||
| .select("status, paystack_reference, paystack_response, error_message") | ||
| .eq("idempotency_key", idempotencyKey) | ||
| .eq("user_id", userId) | ||
| .maybeSingle(); | ||
|
|
||
| if (existing?.status === "pending" && existing.paystack_reference) { | ||
| console.log(`${LOG} idempotency cache pending`, { | ||
| reference: existing.paystack_reference, | ||
| }); | ||
| return new Response( | ||
| JSON.stringify({ | ||
| outcome: "pending", | ||
| reference: existing.paystack_reference, | ||
| cached: true, | ||
| }), | ||
| { status: 200, headers: { ...corsHeaders, "Content-Type": "application/json" } } | ||
| ); | ||
| } | ||
|
|
||
| if (existing?.status === "success") { | ||
| console.log(`${LOG} idempotency cache success`, { | ||
| reference: existing.paystack_reference, | ||
| }); | ||
| return new Response( | ||
| JSON.stringify({ | ||
| outcome: "success", | ||
| reference: existing.paystack_reference, | ||
| cached: true, | ||
| }), | ||
| { status: 200, headers: { ...corsHeaders, "Content-Type": "application/json" } } | ||
| ); | ||
| } | ||
|
|
||
| if (existing?.status === "failed") { | ||
| console.log(`${LOG} idempotency cache failed`, { | ||
| reference: existing.paystack_reference, | ||
| message: existing.error_message, | ||
| }); | ||
| return new Response( | ||
| JSON.stringify({ | ||
| outcome: "failed", | ||
| reference: existing.paystack_reference, | ||
| message: existing.error_message, | ||
| cached: true, | ||
| }), | ||
| { status: 200, headers: { ...corsHeaders, "Content-Type": "application/json" } } | ||
| ); | ||
| } | ||
|
|
||
| const planConfig = PLAN_MOMO_CHARGE[planKey]; | ||
| const billingCycle = planKey.includes("quarterly") ? "quarterly" : "monthly"; | ||
|
|
||
| const { data: profile } = await admin | ||
| .from("profiles") | ||
| .select("email") | ||
| .eq("id", userId) | ||
| .single(); | ||
|
|
||
| const email = | ||
| profile?.email || | ||
| userData.user.email || | ||
| `${userId}@cediwise.phone`; | ||
|
|
||
| const emailSource = profile?.email | ||
| ? "profile" | ||
| : userData.user.email | ||
| ? "auth_user" | ||
| : "synthetic"; | ||
|
|
||
| const displayName = await resolveCustomerDisplayName(admin, userId); | ||
|
|
||
| console.log(`${LOG} charge context`, { | ||
| planKey, | ||
| billingCycle, | ||
| amountPesewas: planConfig.amount_pesewas, | ||
| emailSource, | ||
| emailDomain: email.includes("@") ? email.split("@")[1] : "?", | ||
| hasCustomerDisplayName: !!displayName, | ||
| }); | ||
|
|
||
| if (!existing) { | ||
| console.log(`${LOG} idempotency insert new row`); | ||
| const { error: insErr } = await admin.from("momo_charge_attempts").insert({ | ||
| idempotency_key: idempotencyKey, | ||
| user_id: userId, | ||
| plan_key: planKey, | ||
| status: "pending", | ||
| }); | ||
| if (insErr) { | ||
| console.error(`${LOG} idempotency insert failed`, insErr); | ||
| return new Response(JSON.stringify({ error: "Could not start charge" }), { | ||
| status: 500, | ||
| headers: { ...corsHeaders, "Content-Type": "application/json" }, | ||
| }); | ||
| } | ||
| } else { | ||
| console.log(`${LOG} idempotency retry pending row (no reference yet)`); | ||
| } |
There was a problem hiding this comment.
Treat an existing pending row as in-flight, not as permission to create another charge.
The retry path only short-circuits once paystack_reference is set. A second request that lands after the row insert but before the first Paystack response falls through to lines 320-322 and calls /charge again, so the same idempotency key can create multiple external charge attempts. Return the existing pending state immediately, and handle insert races the same way instead of returning a 500.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/paystack-momo-charge/index.ts` around lines 223 - 322, The
code treats a pending row with no paystack_reference as eligible to create a new
charge; change the idempotency logic so that if existing?.status === "pending"
you immediately return the pending outcome (cached: true) regardless of
paystack_reference to mark it as in-flight; additionally, when inserting into
admin.from("momo_charge_attempts") with idempotencyKey fails (insErr), detect
unique/constraint race errors and re-query the same table (using the same
idempotencyKey/maybeSingle) to return the existing pending row instead of
returning a 500, so concurrent requests don’t trigger duplicate external /charge
calls.
| await admin | ||
| .from("momo_charge_attempts") | ||
| .update({ | ||
| status: "failed", | ||
| paystack_response: paystackData, | ||
| error_message: msg, | ||
| updated_at: new Date().toISOString(), | ||
| }) | ||
| .eq("idempotency_key", idempotencyKey); | ||
|
|
||
| return new Response( | ||
| JSON.stringify({ outcome: "failed", message: msg }), | ||
| { status: 400, headers: { ...corsHeaders, "Content-Type": "application/json" } } | ||
| ); | ||
| } | ||
|
|
||
| console.log(`${LOG} Paystack accepted charge init`, { | ||
| reference, | ||
| paystackDataStatus: payStatus, | ||
| }); | ||
|
|
||
| await admin | ||
| .from("momo_charge_attempts") | ||
| .update({ | ||
| paystack_reference: reference ?? null, | ||
| paystack_response: paystackData, | ||
| status: payStatus === "success" ? "success" : "pending", | ||
| updated_at: new Date().toISOString(), | ||
| }) | ||
| .eq("idempotency_key", idempotencyKey); |
There was a problem hiding this comment.
Keep attempt writes scoped to (idempotency_key, user_id).
Both update statements filter only on idempotency_key, even though the read path keys attempts by idempotency_key plus user_id. If the underlying uniqueness is composite, one caller can mutate another caller's row when keys collide or are replayed. Add .eq("user_id", userId) to both writes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/paystack-momo-charge/index.ts` around lines 391 - 420, The
two update calls against the "momo_charge_attempts" table (the
admin.from(...).update(...) chains that currently .eq("idempotency_key",
idempotencyKey)) must also scope by user_id to match the read keys; add
.eq("user_id", userId) to both update query chains so they filter on both
idempotencyKey and userId (use the existing idempotencyKey and userId variables
referenced in this function) to prevent one caller from mutating another
caller's row.
| const { data: attempt } = await admin | ||
| .from("momo_charge_attempts") | ||
| .select("user_id, paystack_reference") | ||
| .eq("user_id", userId) | ||
| .eq("paystack_reference", reference) | ||
| .maybeSingle(); | ||
|
|
||
| if (!attempt) { | ||
| return new Response(JSON.stringify({ error: "Reference not found" }), { | ||
| status: 404, | ||
| headers: { ...corsHeaders, "Content-Type": "application/json" }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Handle momo_charge_attempts lookup errors explicitly.
The maybeSingle() error is ignored here, so a database failure currently falls through to 404 Reference not found. Return a 5xx on lookup errors so polling does not mask operational problems as a missing reference.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/paystack-momo-status/index.ts` around lines 76 - 88, The
DB lookup using admin.from("momo_charge_attempts").select(...).maybeSingle()
currently ignores the returned error and treats failures as a missing reference;
update the code that calls maybeSingle() (the variable destructuring around
attempt) to also capture the error (e.g., { data: attempt, error }) and if error
is non-null return a 5xx Response (include corsHeaders and Content-Type like the
404 branch) with an explanatory error payload; only proceed to the 404 branch
when error is null and attempt is falsy.
| create or replace view "public"."module_completion_stats" as SELECT module_id, | ||
| count(DISTINCT user_id) AS total_users, | ||
| avg( | ||
| CASE | ||
| WHEN (event_type = 'module_completed'::text) THEN 1 | ||
| ELSE 0 | ||
| END) AS completion_rate, | ||
| count(*) FILTER (WHERE (event_type = 'lesson_viewed'::text)) AS total_views, | ||
| count(*) FILTER (WHERE (event_type = 'quiz_attempted'::text)) AS total_quiz_attempts | ||
| FROM public.literacy_events | ||
| WHERE (module_id IS NOT NULL) | ||
| GROUP BY module_id; |
There was a problem hiding this comment.
completion_rate is event-weighted instead of user-weighted.
A module with lots of lesson views will look less “complete” even if every learner finished, because every non-module_completed event contributes a zero. This should be based on distinct users who completed ÷ distinct users who engaged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260421193238_remote_schema.sql` around lines 165 - 176,
The view module_completion_stats currently computes completion_rate using
avg(CASE ...) which weights by events; change it to a user-based ratio by
replacing that expression with: the count of DISTINCT user_id filtered where
event_type = 'module_completed' divided by the count of DISTINCT user_id (i.e.,
distinct users who engaged the module), casting to numeric and wrapping the
denominator in NULLIF(...,0) to avoid division-by-zero; keep the rest of the
view (module_id, total_users, total_views, total_quiz_attempts) and use
public.literacy_events with its module_id, user_id, and event_type filters to
implement this.
| create or replace function public.log_subscription_change () returns trigger | ||
| language plpgsql | ||
| security definer | ||
| set search_path = public | ||
| as $$ | ||
| declare | ||
| event text; | ||
| old_tier text; | ||
| new_tier text; | ||
| old_status text; | ||
| new_status text; | ||
| begin | ||
| old_tier := coalesce(OLD.plan, 'free'); | ||
| new_tier := coalesce(NEW.plan, 'free'); | ||
| old_status := coalesce(OLD.status, 'active'); | ||
| new_status := coalesce(NEW.status, 'active'); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PostgreSQL PL/pgSQL trigger functions, is OLDavailable duringINSERTtriggers, or does referencingOLD.columnbefore checkingTG_OP cause a runtime error?
💡 Result:
In PostgreSQL PL/pgSQL trigger functions, OLD is null and not available during INSERT triggers. Referencing OLD.column before checking TG_OP will cause a runtime error: "record "old" is not assigned yet". The official documentation states: "OLD: Data type RECORD; variable holding the old database row for UPDATE/DELETE operations in row-level triggers. This variable is null in statement-level triggers and for INSERT operations." Multiple sources confirm this behavior, including error examples from Stack Overflow and DBA Stack Exchange where accessing OLD in INSERT triggers fails with the exact error message.
Citations:
- 1: https://www.postgresql.org/docs/current/plpgsql-trigger.html
- 2: https://www.postgresql.org/message-id/m3aauaojfb.fsf@passepartout.tim-landscheidt.de
- 3: https://stackoverflow.com/questions/40752230/record-old-not-assigned-yet
- 4: https://dba.stackexchange.com/questions/253699/trigger-function-error-record-old-not-yet-assigned
- 5: http://www.postgresql.org/docs/current/static/plpgsql-trigger.html
🏁 Script executed:
cat -n supabase/migrations/20260422171000_subscription_billing_v2.sql | sed -n '130,160p'Repository: teebhagg/CediWise
Length of output: 1183
Move OLD dereferencing inside the UPDATE/DELETE branch.
Lines 146 and 148 access OLD.plan and OLD.status before checking TG_OP. On INSERT triggers, OLD is null, so this will fail with "record 'old' is not assigned yet" at runtime.
Suggested fix
create or replace function public.log_subscription_change () returns trigger
language plpgsql
security definer
set search_path = public
as $$
declare
event text;
old_tier text;
new_tier text;
old_status text;
new_status text;
begin
- old_tier := coalesce(OLD.plan, 'free');
new_tier := coalesce(NEW.plan, 'free');
- old_status := coalesce(OLD.status, 'active');
new_status := coalesce(NEW.status, 'active');
+
+ if tg_op = 'UPDATE' then
+ old_tier := coalesce(OLD.plan, 'free');
+ old_status := coalesce(OLD.status, 'active');
+ end if;📝 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.
| create or replace function public.log_subscription_change () returns trigger | |
| language plpgsql | |
| security definer | |
| set search_path = public | |
| as $$ | |
| declare | |
| event text; | |
| old_tier text; | |
| new_tier text; | |
| old_status text; | |
| new_status text; | |
| begin | |
| old_tier := coalesce(OLD.plan, 'free'); | |
| new_tier := coalesce(NEW.plan, 'free'); | |
| old_status := coalesce(OLD.status, 'active'); | |
| new_status := coalesce(NEW.status, 'active'); | |
| create or replace function public.log_subscription_change () returns trigger | |
| language plpgsql | |
| security definer | |
| set search_path = public | |
| as $$ | |
| declare | |
| event text; | |
| old_tier text; | |
| new_tier text; | |
| old_status text; | |
| new_status text; | |
| begin | |
| new_tier := coalesce(NEW.plan, 'free'); | |
| new_status := coalesce(NEW.status, 'active'); | |
| if tg_op = 'UPDATE' then | |
| old_tier := coalesce(OLD.plan, 'free'); | |
| old_status := coalesce(OLD.status, 'active'); | |
| end if; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260422171000_subscription_billing_v2.sql` around lines
134 - 149, In the log_subscription_change trigger function you must not
dereference OLD on INSERT; move assignments that use OLD (old_tier :=
coalesce(OLD.plan, 'free') and old_status := coalesce(OLD.status, 'active'))
into the branch that handles UPDATE/DELETE (i.e., when TG_OP = 'UPDATE' or
'DELETE'), and similarly ensure assignments using NEW (new_tier :=
coalesce(NEW.plan, 'free') and new_status := coalesce(NEW.status, 'active'))
occur only in the INSERT/UPDATE branch (when NEW is present); update the
conditional branches in log_subscription_change to set old_tier/old_status and
new_tier/new_status in the appropriate TG_OP-specific blocks so no NULL record
is dereferenced.
| elsif new_status = 'pending_payment' then | ||
| event := 'subscription_activated'; | ||
| elsif new_status = 'active' and new_tier != 'free' then | ||
| event := 'subscription_activated'; | ||
| else | ||
| event := 'subscription_activated'; |
There was a problem hiding this comment.
pending_payment should not be logged as subscription_activated.
This marks unpaid subscriptions as activated, which will skew conversion metrics and any downstream logic keyed off the activity log. Either skip logging here or add a distinct pending-payment event.
Suggested fix
- elsif new_status = 'pending_payment' then
- event := 'subscription_activated';
+ elsif new_status = 'pending_payment' then
+ return NEW;📝 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.
| elsif new_status = 'pending_payment' then | |
| event := 'subscription_activated'; | |
| elsif new_status = 'active' and new_tier != 'free' then | |
| event := 'subscription_activated'; | |
| else | |
| event := 'subscription_activated'; | |
| elsif new_status = 'pending_payment' then | |
| return NEW; | |
| elsif new_status = 'active' and new_tier != 'free' then | |
| event := 'subscription_activated'; | |
| else | |
| event := 'subscription_activated'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260422171000_subscription_billing_v2.sql` around lines
163 - 168, The branch assigning event for status transitions incorrectly maps
new_status = 'pending_payment' to 'subscription_activated'; update the logic in
the trigger/function that sets the local variable event (the code using
new_status, new_tier and event) so that 'pending_payment' either does not
generate an activity log (skip/return) or sets a distinct event name like
'subscription_pending_payment' instead of 'subscription_activated', and ensure
downstream callers that rely on event are adjusted accordingly.
| begin | ||
| old_tier := coalesce(OLD.plan, 'free'); | ||
| new_tier := coalesce(NEW.plan, 'free'); | ||
| old_status := coalesce(OLD.status, 'active'); | ||
| new_status := coalesce(NEW.status, 'active'); | ||
|
|
||
| if tg_op = 'INSERT' then |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PostgreSQL row-level INSERT triggers, is OLDundefined, and does accessingOLD.columnbefore branching onTG_OP raise an error such as "record OLD is not assigned yet"?
💡 Result:
Yes. In a row-level trigger fired for INSERT, the special trigger variable OLD is not assigned (NULL/unavailable), because OLD “holds the old database row for UPDATE/DELETE operations … [and] is null … for INSERT operations”. Therefore, directly accessing OLD.column in an INSERT trigger can raise an error like “record "old" is not assigned yet” (the typical DETAIL is “The tuple structure of a not-yet-assigned record is indeterminate.”). If you branch on TG_OP, it is safe only if you ensure you do not reference OLD.column in the INSERT path. However, you should not rely on boolean expression short-circuiting inside complex expressions (e.g., using OLD inside an OR/AND condition that also tests TG_OP), because PostgreSQL does not guarantee evaluation order of subexpressions. A robust pattern is an explicit IF TG_OP = 'INSERT' ... ELSE ... block and only use OLD.* inside the non-INSERT branch. Grounding: PostgreSQL docs state OLD is null for INSERT operations; and community reports show the exact “record OLD is not assigned yet” error when OLD is referenced in INSERT triggers.
Citations:
- 1: http://www.postgresql.org/docs/16/plpgsql-trigger.html
- 2: https://www.postgresql.org/docs/current/plpgsql-trigger.html
- 3: https://dba.stackexchange.com/questions/253699/trigger-function-error-record-old-not-yet-assigned
- 4: https://stackoverflow.com/questions/40752230/record-old-not-assigned-yet
- 5: https://dba.stackexchange.com/questions/313847/why-this-trigger-is-raising-record-old-is-not-assigned-yet-exception
🏁 Script executed:
cat -n supabase/migrations/20260424160000_billing_cycle_proration.sql | head -120 | tail -40Repository: teebhagg/CediWise
Length of output: 1435
Move OLD dereference inside the UPDATE branch to avoid "record OLD is not assigned yet" error on INSERT.
This trigger accesses OLD.plan and OLD.status at lines 92–94 before checking TG_OP. In PostgreSQL row-level INSERT triggers, OLD is unavailable, so every new subscription insert will fail with "record 'old' is not assigned yet" before any event logging occurs.
Assign old_tier and old_status only within the UPDATE branch (after line 117). In the INSERT branch, initialize them to null or derive them from NEW only.
Suggested change
begin
- old_tier := coalesce(OLD.plan, 'free');
new_tier := coalesce(NEW.plan, 'free');
- old_status := coalesce(OLD.status, 'active');
new_status := coalesce(NEW.status, 'active');
if tg_op = 'INSERT' then
+ old_tier := null;
+ old_status := null;
if new_status = 'trial' then
elsif tg_op = 'UPDATE' then
+ old_tier := coalesce(OLD.plan, 'free');
+ old_status := coalesce(OLD.status, 'active');
if OLD.plan is not distinct from NEW.plan🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260424160000_billing_cycle_proration.sql` around lines
91 - 97, The trigger currently dereferences OLD (old_tier := coalesce(OLD.plan,
'free'); old_status := coalesce(OLD.status, 'active')) before checking TG_OP,
which causes "record OLD is not assigned yet" on INSERT; move those OLD-based
assignments into the UPDATE branch (after the TG_OP check) and in the INSERT
branch set old_tier and old_status to NULL or derive from NEW (e.g., new_tier :=
coalesce(NEW.plan,'free')) so only UPDATE uses OLD; update any logic that
references old_tier/old_status to account for NULL in INSERT.
| -- ============================================================================ | ||
| -- GENERATED FILE — DO NOT EDIT BY HAND | ||
| -- Regenerate: supabase db dump --linked --schema public -f supabase/schema.generated.sql |
There was a problem hiding this comment.
Regenerate this schema dump after applying the billing v2 migrations.
This snapshot still shows the pre-v2 subscriptions shape and old subscription_activity_log enum set, and it omits the new billing tables entirely. Anyone restoring or diffing from supabase/schema.generated.sql will end up with a schema that does not match the migrations in this PR.
Also applies to: 986-1023
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/schema.generated.sql` around lines 1 - 3, The generated schema
snapshot is out of date: it still contains the pre-v2 "subscriptions" table
shape and the old "subscription_activity_log" enum values and is missing the new
billing tables; regenerate the SQL dump after applying the billing v2 migrations
so the file reflects the current schema (run the same command used to create it:
"supabase db dump --linked --schema public -f supabase/schema.generated.sql" or
your project's canonical dump step) and commit the updated
supabase/schema.generated.sql so "subscriptions", "subscription_activity_log"
and the new billing tables appear correctly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/README.md`:
- Around line 9-10: Update the README.md wording to state that the drift-fix
migration (99999999999999_fix_schema_drift_if_missing.sql) must be moved from
supabase/_stash into supabase/migrations before the first supabase db push, and
that the file should be renamed to a timestamp that sorts after the project
baseline (e.g. 20300101000000_...) if db pull generated a newer timestamp;
reference the migration filename
(99999999999999_fix_schema_drift_if_missing.sql), the stash path
(supabase/_stash) and the migrations folder (supabase/migrations) so
contributors know to physically relocate and optionally rename the file prior to
pushing.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8d0481d9-c905-4274-9741-e1327efbe807
📒 Files selected for processing (1)
supabase/migrations/README.md
| 4. **Drift fix** — `99999999999999_fix_schema_drift_if_missing.sql` is already in this folder; it sorts **after** any real pull timestamp from 2026–2099. | ||
| If `db pull` generated a migration with a timestamp *newer* than `99999999999999`, rename this file to a timestamp **after** the baseline (e.g. `20300101000000_...`) before first `db push`. |
There was a problem hiding this comment.
Drift-fix migration location is inconsistent with the documented push flow.
This section says the drift-fix SQL is already in this folder, but the file is at supabase/_stash/99999999999999_fix_schema_drift_if_missing.sql. With the current wording, contributors can run supabase db push without ever moving it into supabase/migrations, so the corrective migration may never execute.
Suggested doc fix
-4. **Drift fix** — `99999999999999_fix_schema_drift_if_missing.sql` is already in this folder; it sorts **after** any real pull timestamp from 2026–2099.
- If `db pull` generated a migration with a timestamp *newer* than `99999999999999`, rename this file to a timestamp **after** the baseline (e.g. `20300101000000_...`) before first `db push`.
+4. **Drift fix** — copy `supabase/_stash/99999999999999_fix_schema_drift_if_missing.sql` into `supabase/migrations/` before first push.
+ Name it with a timestamp **after** your baseline migration (e.g. `20300101000000_fix_schema_drift_if_missing.sql`) so it runs in order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/README.md` around lines 9 - 10, Update the README.md
wording to state that the drift-fix migration
(99999999999999_fix_schema_drift_if_missing.sql) must be moved from
supabase/_stash into supabase/migrations before the first supabase db push, and
that the file should be renamed to a timestamp that sorts after the project
baseline (e.g. 20300101000000_...) if db pull generated a newer timestamp;
reference the migration filename
(99999999999999_fix_schema_drift_if_missing.sql), the stash path
(supabase/_stash) and the migrations folder (supabase/migrations) so
contributors know to physically relocate and optionally rename the file prior to
pushing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb663b7687
ℹ️ 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".
| const supabase = createClient(SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY, { | ||
| auth: { persistSession: false }, | ||
| }); |
There was a problem hiding this comment.
This function never checks the caller’s identity/role before using the service-role client to send push notifications, so any authenticated user who can invoke Edge Functions can trigger broadcasts by passing a known campaign_id. That is especially exploitable because authenticated users are granted read access to sent campaign rows (announcement_campaigns_user_read_sent in supabase/migrations/20260422162200_feedback_mobile_and_announcement_reads.sql), which exposes valid IDs to replay. Please gate this endpoint to admins (or require an internal secret) before executing the send path.
Useful? React with 👍 / 👎.
| if (!userId || !planKey) { | ||
| return permanentError("charge.success: missing userId or planKey in metadata"); | ||
| } |
There was a problem hiding this comment.
The handler hard-fails when userId/planKey are missing in metadata, even though recurring charges can arrive without those fields; in that case it returns early and skips all subscription period updates. Because renewal success is then ignored, current_period_end/next_billing_date are not advanced and later janitor logic can incorrectly move paying users into grace/downgrade paths based on stale dates. Add a fallback lookup path (for example via subscription/customer codes on the payload) before treating the event as permanently ignorable.
Useful? React with 👍 / 👎.
…://github.com/teebhagg/CediWise into feature/billing-redesign-and-sme-enhancements
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca0aaeccd8
ℹ️ 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".
| } else { | ||
| console.log(`${LOG} idempotency retry pending row (no reference yet)`); | ||
| } |
There was a problem hiding this comment.
This handler only treats a pending attempt as idempotent when paystack_reference is already set, but it falls through and calls POST /charge again when the row is still pending with a null reference. If a client retries quickly (or two requests race) before the first request stores the reference, the same idempotency_key can initiate multiple Paystack charges, which can double-charge users instead of returning a cached pending outcome.
Useful? React with 👍 / 👎.
| await supabase | ||
| .from("announcement_campaigns") | ||
| .update({ status: "sending" }) | ||
| .eq("id", row.id); |
There was a problem hiding this comment.
The function always flips the campaign to sending and proceeds, regardless of its current status. A replay of the same campaign_id (for example, after a caller timeout/retry) will resend push notifications to the same audience because there is no queued-only transition check or idempotency gate before dispatch.
Useful? React with 👍 / 👎.
| elsif old_status <> 'expired' and new_status = 'expired' then | ||
| event := 'subscription_cancelled'; |
There was a problem hiding this comment.
This migration redefines log_subscription_change and routes every non-expired→expired transition into subscription_cancelled. Because the explicit grace_period -> expired/free => auto_downgraded branch from the earlier billing migration is missing here, janitor-driven expirations are misclassified, which breaks downstream metrics/alerts that rely on auto_downgraded events.
Useful? React with 👍 / 👎.
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af35f17201
ℹ️ 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".
| email: userEmail.trim().toLowerCase(), | ||
| is_beta: false, | ||
| version: appVersion, | ||
| source: "mobile_app_home_prompt", |
There was a problem hiding this comment.
This insert uses source: "mobile_app_home_prompt", but the only authenticated insert policy added for public.feedback allows source = 'mobile_app' (see supabase/migrations/20260422162200_feedback_mobile_and_announcement_reads.sql). In production, quick-rating submissions from this modal will be rejected by RLS every time, so users can never submit feedback through this path.
Useful? React with 👍 / 👎.
| const { error } = await supabase.from("feedback").insert({ | ||
| category, | ||
| rating, | ||
| feedback_text: trimmed, | ||
| email: email.toLowerCase(), |
There was a problem hiding this comment.
The submit handler sends the insert immediately but never flips submitting to true, while the button disable/loading logic depends on that state. On slow networks a user can tap submit repeatedly and create duplicate feedback rows because there is no client-side in-flight guard or server-side idempotency here.
Useful? React with 👍 / 👎.
… payment reconciliation
|
❌ TypeScript errors ( |
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
CediWise/supabase/functions/daily-cash-flow-check/index.ts
Lines 97 to 101 in 9df9b02
Use the real announcement_deliveries columns in cash-flow job
daily-cash-flow-check reads and writes announcement_deliveries with user_id, announcement_id, and delivered_at, but this migration set defines announcement_deliveries with campaign_id, push_device_id, expo_push_token, status, and created_at instead. As written, the cooldown lookup and insert will fail at runtime with missing-column errors, so scheduled runs cannot complete the notification flow.
ℹ️ 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".
| Deno.serve(async (req) => { | ||
| if (req.method === "OPTIONS") { | ||
| return new Response("ok", { headers: corsHeaders }); | ||
| } |
There was a problem hiding this comment.
This handler performs global subscription mutations with a service-role client but never authenticates or authorizes the caller, so any valid authenticated token can trigger grace-period and expiry processing on demand. That makes a maintenance endpoint user-invokable and allows non-admin users to run account-wide billing state transitions outside the intended schedule.
Useful? React with 👍 / 👎.
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4da29ff792
ℹ️ 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".
| await supabase | ||
| .from("announcement_campaigns") | ||
| .update({ | ||
| status: "sent", |
There was a problem hiding this comment.
The final campaign update unconditionally writes status: "sent" even when every Expo send attempt failed (success === 0 and failure > 0). In that scenario the row is misclassified as delivered, which hides operational failures and blocks any workflow that retries only failed campaigns. Update the status based on delivery outcome (at least when all attempts fail) so failed broadcasts are visible and recoverable.
Useful? React with 👍 / 👎.
… and GitHub Actions overview; add AgooSMS test script
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b069c16325
ℹ️ 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".
| user_id: userId, | ||
| plan: subPlan, | ||
| status: subStatus, |
There was a problem hiding this comment.
The normal charge.success branch writes plan: subPlan directly from webhook metadata without verifying that data.amount (or plan code) matches the expected price for that tier/cadence. Since checkout is initiated client-side (cediwise-mobile-app/app/upgrade.tsx) and submits both amount and metadata from the device, a tampered client can underpay and still set planKey to a higher tier, and this handler will activate it. Add a server-side amount/plan integrity check before the subscription upsert.
Useful? React with 👍 / 👎.
| if (row.audience_type === "single_user" && row.target_user_id) { | ||
| deviceQuery = deviceQuery.eq("user_id", row.target_user_id); |
There was a problem hiding this comment.
This filter only narrows to one user when target_user_id is present; otherwise a single_user campaign falls through to the default query and sends to all active devices. That state is reachable because announcement_campaigns.target_user_id can become null (ON DELETE SET NULL in the schema), so deleting the target account after queueing can accidentally broadcast a private message. Fail fast when audience_type is single_user and target_user_id is null.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@cediwise-dashboard/app/`(dashboard)/announcements/announcement-delete-button.tsx:
- Around line 29-40: The handleDelete function can throw during
deleteAnnouncementCampaign and never reach setLoading(false); wrap the await
call in a try/catch/finally inside handleDelete: call setLoading(true) then try
{ const result = await deleteAnnouncementCampaign(campaignId); if (!result.ok)
setError(result.error ?? "Could not delete campaign"); else { setOpen(false);
router.refresh(); } } catch (err) { setError(String(err) || "Could not delete
campaign"); } finally { setLoading(false); } to ensure loading is always reset
and provide a fallback error message; reference handleDelete,
deleteAnnouncementCampaign, setLoading, setError, setOpen, and router.refresh
when applying the change.
In `@cediwise-dashboard/app/`(dashboard)/feedback/page.tsx:
- Line 27: The active-filter UI logic fails to consider the parsed source
filter; update the hasActiveFilters check in feedback-table.tsx to include
filters.source (or equivalent local variable) so that when source is set (e.g.,
?source=mobile_app) the UI marks filters as active; specifically, add a truthy
check for filters.source alongside the existing checks in the hasActiveFilters
computation and ensure any clear-all or pill-rendering logic also accounts for
filters.source.
In `@cediwise-dashboard/lib/actions/announcements.ts`:
- Around line 235-236: The JSDoc "Push announcement to a single user’s
registered devices only." is stale and misplaced above
deleteAnnouncementCampaign; remove that JSDoc or move it so it sits immediately
above the sendAnnouncementToUser function. Update the comment block so
deleteAnnouncementCampaign only has its relevant JSDoc ("Permanently remove a
campaign...") and sendAnnouncementToUser has the single-user push JSDoc,
ensuring the comment-to-function association matches the symbols
deleteAnnouncementCampaign and sendAnnouncementToUser.
In `@cediwise-dashboard/scripts/test-agoosms.mjs`:
- Line 39: The script currently falls back to a hard-coded phone number via the
const to = process.argv[2] ?? "+233539672914"; which risks accidental sends;
change this so the recipient must be provided explicitly (either via
process.argv[2] or a dedicated test env var like TEST_SMS_TO) and fail fast if
neither is set—update the logic around the to variable in test-agoosms.mjs (the
process.argv handling) to check for an explicit value, throw or exit with a
clear error message when missing, and remove the hard-coded fallback.
- Around line 51-63: Wrap the fetch call in an AbortController timeout and a
try/catch to handle network errors and prevent hangs: create an AbortController,
start a setTimeout to call controller.abort() after a chosen timeout (e.g.,
10s), pass controller.signal to fetch(API_URL, { ..., signal }), then await
fetch inside try/catch; on success read res.text(), clear the timeout, log
status and body, and exit 0 for res.ok or 1 otherwise; on catch log the error
(including abort/errors) and exit non‑zero. Reference symbols: API_URL, apiKey,
to, message, fetch call returning res, res.text(), and process.exit.
In `@cediwise-mobile-app/app/feedback.tsx`:
- Around line 107-112: The call to recordFullFeedbackScreenSubmitted should be
best-effort and must not block the success path: stop awaiting it (or run it in
a detached try/catch/promise chain) so failures don't prevent showSuccess,
Haptics.notificationAsync, setSubmitting(false) and router.back() from running;
locate the block containing recordFullFeedbackScreenSubmitted(user.id) in
feedback.tsx and either call it without await or call it and attach .catch(...)
to swallow/log errors instead of throwing so the rest of the success flow
(showSuccess, Haptics.notificationAsync, setSubmitting, router.back) always
executes.
- Around line 69-123: onSubmit currently never sets the submitting flag so
multiple taps can cause duplicate inserts; update the onSubmit handler to first
check the submitting state and return early if true, then call
setSubmitting(true) at the very start of the async routine and ensure
setSubmitting(false) is called in a finally block so the flag is always cleared;
modify references inside onSubmit (function name onSubmit, state setter
setSubmitting, and submitting state) so the guard prevents concurrent
submissions while preserving existing error/success flows and cleanup.
In `@cediwise-mobile-app/app/profile/edit.tsx`:
- Around line 49-56: getInitials can throw when given a whitespace-only string
because name.trim() becomes empty but the function still indexes parts[0][0];
update getInitials to trim the input first, check for an empty trimmed string
and return "U" if empty, then split the trimmed value and safely extract the
first character(s) (e.g., guard parts[0] and parts[1] exist before indexing) so
.toUpperCase() is only called on defined characters; keep the same behavior of
returning two-letter initials when two words exist.
In `@cediwise-mobile-app/app/upgrade.tsx`:
- Around line 328-329: The finally block that calls setIsProcessing(false) after
invoking popup.checkout() is premature because popup.checkout() returns
immediately; remove clearing isProcessing from that finally and instead
setIsProcessing(false) only in the payment lifecycle callbacks (onCancel and
onPaymentSuccess) so the UI remains disabled while the checkout is live; update
both occurrences (the block around popup.checkout() and the similar logic in the
other upgrade/downgrade flow) to rely on those handlers to re-enable the CTAs
and ensure no code path leaves isProcessing stuck true on terminal events.
In `@cediwise-mobile-app/components/features/budget/BudgetEngineModeList.tsx`:
- Around line 52-55: The error toast in BudgetEngineModeList.tsx currently says
the update failed while budget.updateBudgetEngineMode (see useBudget.ts) already
persists the new mode locally; change the UX and logging to reflect a sync
failure rather than a loss of the change: in the catch block of
BudgetEngineModeList.tsx keep log.error("Budget engine mode update failed:", e)
for diagnostics but replace the showError call with a message like "Sync failed
— change saved locally" (or use showWarning) so users understand the mode was
applied locally but failed to sync, and ensure any wording references "sync"
rather than "update" to match budget.updateBudgetEngineMode behavior.
- Around line 47-50: The handler that updates mode (the async (mode: (typeof
OPTIONS)[number]["value"]) => { ... }) can still be invoked twice before state
re-renders; add an in-flight ref (e.g., isUpdatingRef via
React.useRef<boolean>(false)) and check it alongside loading/currentMode at the
top (if (loading || currentMode === mode || isUpdatingRef.current) return), set
isUpdatingRef.current = true immediately before starting the async work, and
clear it in the finally block (alongside calling setLoading(false)) so the
operation is atomic per tap burst; reference the existing symbols loading,
currentMode, setLoading and OPTIONS when adding the ref logic.
In `@cediwise-mobile-app/distribution/0.2.6.txt`:
- Around line 1-6: The release notes in distribution/<version>.txt
(distribution/0.2.6.txt) exceed the CI 500-character limit; shorten the content
to a single concise release summary ≤500 characters by compressing or combining
the listed bullets (SME transactions, Home dashboard, Notifications,
Subscriptions/upgrades, Profile, Feedback) into a few short phrases, or move the
full details to the changelog and leave a trimmed summary here, then save the
updated distribution/0.2.6.txt so CI release validation passes.
In `@cediwise-mobile-app/hooks/usePeriodicFeedbackPrompt.ts`:
- Around line 256-264: The insert into the feedback table in
usePeriodicFeedbackPrompt.ts uses source: "mobile_app_home_prompt", which
violates the RLS policy that only allows authenticated inserts for source =
'mobile_app'; update the payload passed to supabase.from("feedback").insert(...)
to use the RLS-allowed value "mobile_app" (or derive from a single shared
constant) instead of "mobile_app_home_prompt" so the insert succeeds under the
existing policy.
- Around line 270-279: The local cooldown update (calls to
loadFeedbackPromptState and saveFeedbackPromptState) is currently allowed to
throw and cause the hook to return { ok: false } after the Supabase row was
already inserted; make these operations best-effort so the submit outcome
remains successful and the UI is dismissed. Wrap the FORCE_PROMPT_EACH_FOCUS_DEV
branch that calls loadFeedbackPromptState and saveFeedbackPromptState in a
try/catch, swallow or log errors (but do not rethrow), and ensure
dismissUiOnly() and the successful return path still execute even if those
functions fail. Preserve existing behavior when FORCE_PROMPT_EACH_FOCUS_DEV is
true and only adjust the error handling around
loadFeedbackPromptState/saveFeedbackPromptState so duplicate rows cannot be
caused by retrying due to a failed local write.
In `@cediwise-mobile-app/utils/smeExport.ts`:
- Around line 10-15: csvCell currently escapes quotes/commas/newlines but
doesn’t neutralize spreadsheet formulas; update csvCell to detect when the raw
string begins with any of the formula-trigger characters (=, +, -, @) and prefix
the value with a neutralizing apostrophe (') before performing the existing
quote-escaping/quoting logic so the cell is treated as text in Excel/Sheets;
implement this check inside the csvCell function (use the raw string from
String(value) and only prefix when the first character is one of those
triggers), then continue with the existing replacement of internal double-quotes
and wrapping in double-quotes when needed.
In `@cediwise-mobile-app/utils/subscriptionReconciliation.ts`:
- Around line 97-134: The timeout currently starts after the initial
fetchSubscriptionRow(...) call so a stalled first fetch can block indefinitely;
move the timeout setup (using timeoutId and timeoutMs) to execute before the
first fetch and before creating the realtime channel/polling so the overall
reconciliation lifecycle is bounded; ensure you still clear timeoutId when
finish(...) is called and that tryResolve(channel, ...) and pollIntervalMs logic
remain unchanged so polling/realtime behavior is identical except the timeout
now covers the initial fetch and subsequent steps; reference
fetchSubscriptionRow, subscriptionReflectsPayment, channel, tryResolve,
timeoutId, timeoutMs, pollIntervalMs, and finish when making the change.
- Around line 9-13: The SubscriptionReconcileRow type (used by the
reconciliation predicate) only includes plan, status, and pending_tier so it
misses paid upgrades that also schedule cadence changes; extend the type to
include the pending_billing_cycle_after (or similarly named) field returned by
billing-upgrade-quote (e.g., pending_billing_cycle_after: string | null) and
update the reconciliation predicate/logic that uses SubscriptionReconcileRow to
require both tier change and matching pending_billing_cycle_after (or absence
thereof) to consider the upgrade complete; ensure any callers that construct
SubscriptionReconcileRow (or the functions using it) populate and check this new
field so cadence changes are not considered applied until
pending_billing_cycle_after matches the expected value.
In `@cediwise-mobile-app/utils/subscriptionSync.ts`:
- Around line 44-50: fetchRow currently swallows Supabase read errors by
returning null, making network/RLS/query failures appear identical to "no row"
and causing long timeouts; change fetchRow (the async function that calls
supabase.from("subscriptions").select(...).eq("user_id", userId).maybeSingle())
to surface errors instead of returning null on failure — specifically check the
response.error and either throw that error or return a distinct Result/union
(e.g., { error }) so callers can abort early and show a payment-sync failure;
update the function's return type and callers accordingly to handle thrown
errors or the error variant rather than treating null as "still syncing".
In `@README.md`:
- Around line 70-92: The fenced project-structure block in README.md is missing
a language tag and triggers markdownlint MD040; update the opening fence for
that tree block (the triple-backtick block that begins the CediWise/ project
tree) to include a language identifier such as "text" (i.e., change ``` to
```text) so the code fence is explicitly typed and the linter warning is
resolved.
In `@supabase/config.toml`:
- Around line 378-380: The subscription-janitor function is exposed because
verify_jwt is false; change the function configuration for subscription-janitor
to require authentication by setting verify_jwt = true (or if public invocation
is intentional, implement a robust shared-secret check inside the
subscription-janitor handler) and keep the existing ENABLE_AUTO_DOWNGRADE
feature flag only as a feature toggle (not as authentication); ensure the
function's code verifies the JWT (or validates the shared secret) before
performing any billing/subscription downgrade operations.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: c64f7d28-d9d2-475f-adea-1a2532868d56
⛔ Files ignored due to path filters (1)
cediwise-mobile-app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
.github/README.mdREADME.mdcediwise-dashboard/app/(dashboard)/announcements/announcement-delete-button.tsxcediwise-dashboard/app/(dashboard)/announcements/announcements-table.tsxcediwise-dashboard/app/(dashboard)/feedback/page.tsxcediwise-dashboard/app/(dashboard)/notifications/page.tsxcediwise-dashboard/lib/actions/announcements.tscediwise-dashboard/lib/actions/feedback.tscediwise-dashboard/scripts/test-agoosms.mjscediwise-mobile-app/app.jsoncediwise-mobile-app/app/(sme)/transactions.tsxcediwise-mobile-app/app/(tabs)/index.tsxcediwise-mobile-app/app/_layout.tsxcediwise-mobile-app/app/budget/settings.tsxcediwise-mobile-app/app/feedback.tsxcediwise-mobile-app/app/notifications/_layout.tsxcediwise-mobile-app/app/notifications/inbox.tsxcediwise-mobile-app/app/notifications/index.tsxcediwise-mobile-app/app/profile/_layout.tsxcediwise-mobile-app/app/profile/edit.tsxcediwise-mobile-app/app/profile/index.tsxcediwise-mobile-app/app/upgrade.tsxcediwise-mobile-app/components/features/budget/BudgetEngineModeList.tsxcediwise-mobile-app/components/feedback/PeriodicFeedbackPromptModal.tsxcediwise-mobile-app/constants/billingPlans.tscediwise-mobile-app/contexts/TierContext.tsxcediwise-mobile-app/distribution/0.2.6.txtcediwise-mobile-app/hooks/usePeriodicFeedbackPrompt.tscediwise-mobile-app/hooks/useSMETransactionFilters.tscediwise-mobile-app/package.jsoncediwise-mobile-app/services/notifications.tscediwise-mobile-app/stores/notificationsStore.tscediwise-mobile-app/utils/auth.tscediwise-mobile-app/utils/authRouting.tscediwise-mobile-app/utils/feedbackPromptStorage.tscediwise-mobile-app/utils/smeExport.tscediwise-mobile-app/utils/subscriptionReconciliation.tscediwise-mobile-app/utils/subscriptionSync.tscediwise-mobile-app/utils/tierGate.tssupabase/README.mdsupabase/config.toml
| async function handleDelete() { | ||
| setLoading(true); | ||
| setError(null); | ||
| const result = await deleteAnnouncementCampaign(campaignId); | ||
| setLoading(false); | ||
| if (!result.ok) { | ||
| setError(result.error ?? "Could not delete campaign"); | ||
| return; | ||
| } | ||
| setOpen(false); | ||
| router.refresh(); | ||
| } |
There was a problem hiding this comment.
Handle thrown delete failures so loading always resets.
At Line 32, a thrown error bypasses Line 33, so loading can stay true and the dialog becomes stuck. Wrap this in try/catch/finally and set a fallback error message.
Suggested fix
async function handleDelete() {
- setLoading(true);
- setError(null);
- const result = await deleteAnnouncementCampaign(campaignId);
- setLoading(false);
- if (!result.ok) {
- setError(result.error ?? "Could not delete campaign");
- return;
- }
- setOpen(false);
- router.refresh();
+ setLoading(true);
+ setError(null);
+ try {
+ const result = await deleteAnnouncementCampaign(campaignId);
+ if (!result.ok) {
+ setError(result.error ?? "Could not delete campaign");
+ return;
+ }
+ setOpen(false);
+ router.refresh();
+ } catch {
+ setError("Could not delete campaign");
+ } finally {
+ setLoading(false);
+ }
}📝 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.
| async function handleDelete() { | |
| setLoading(true); | |
| setError(null); | |
| const result = await deleteAnnouncementCampaign(campaignId); | |
| setLoading(false); | |
| if (!result.ok) { | |
| setError(result.error ?? "Could not delete campaign"); | |
| return; | |
| } | |
| setOpen(false); | |
| router.refresh(); | |
| } | |
| async function handleDelete() { | |
| setLoading(true); | |
| setError(null); | |
| try { | |
| const result = await deleteAnnouncementCampaign(campaignId); | |
| if (!result.ok) { | |
| setError(result.error ?? "Could not delete campaign"); | |
| return; | |
| } | |
| setOpen(false); | |
| router.refresh(); | |
| } catch { | |
| setError("Could not delete campaign"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@cediwise-dashboard/app/`(dashboard)/announcements/announcement-delete-button.tsx
around lines 29 - 40, The handleDelete function can throw during
deleteAnnouncementCampaign and never reach setLoading(false); wrap the await
call in a try/catch/finally inside handleDelete: call setLoading(true) then try
{ const result = await deleteAnnouncementCampaign(campaignId); if (!result.ok)
setError(result.error ?? "Could not delete campaign"); else { setOpen(false);
router.refresh(); } } catch (err) { setError(String(err) || "Could not delete
campaign"); } finally { setLoading(false); } to ensure loading is always reset
and provide a fallback error message; reference handleDelete,
deleteAnnouncementCampaign, setLoading, setError, setOpen, and router.refresh
when applying the change.
| fromDate: searchParams.fromDate, | ||
| toDate: searchParams.toDate, | ||
| search: searchParams.search?.trim() || undefined, | ||
| source: searchParams.source?.trim() || undefined, |
There was a problem hiding this comment.
source filter is parsed, but active-filter UI is not fully wired.
Line 27 adds source, but cediwise-dashboard/app/(dashboard)/feedback/feedback-table.tsx (Lines 107-116) does not include filters.source in hasActiveFilters. With ?source=mobile_app, data is filtered while UI can still indicate no active filters.
Suggested follow-up in feedback-table.tsx
const hasActiveFilters = useMemo(() => {
return !!(
filters.category ||
filters.rating ||
filters.search ||
filters.fromDate ||
filters.toDate ||
+ filters.source ||
typeof filters.isBeta === "boolean"
);
}, [filters]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-dashboard/app/`(dashboard)/feedback/page.tsx at line 27, The
active-filter UI logic fails to consider the parsed source filter; update the
hasActiveFilters check in feedback-table.tsx to include filters.source (or
equivalent local variable) so that when source is set (e.g., ?source=mobile_app)
the UI marks filters as active; specifically, add a truthy check for
filters.source alongside the existing checks in the hasActiveFilters computation
and ensure any clear-all or pill-rendering logic also accounts for
filters.source.
| /** Push announcement to a single user’s registered devices only. */ | ||
| /** Permanently remove a campaign, delivery rows (CASCADE), and mobile read receipts (CASCADE). */ |
There was a problem hiding this comment.
Remove/move the stale JSDoc above the delete action.
Line 235 documents single-user push behavior, but the next symbol is deleteAnnouncementCampaign. This is misleading and should be removed or moved above sendAnnouncementToUser.
Suggested fix
-/** Push announcement to a single user’s registered devices only. */
/** Permanently remove a campaign, delivery rows (CASCADE), and mobile read receipts (CASCADE). */
export async function deleteAnnouncementCampaign(📝 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.
| /** Push announcement to a single user’s registered devices only. */ | |
| /** Permanently remove a campaign, delivery rows (CASCADE), and mobile read receipts (CASCADE). */ | |
| /** Permanently remove a campaign, delivery rows (CASCADE), and mobile read receipts (CASCADE). */ | |
| export async function deleteAnnouncementCampaign( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-dashboard/lib/actions/announcements.ts` around lines 235 - 236, The
JSDoc "Push announcement to a single user’s registered devices only." is stale
and misplaced above deleteAnnouncementCampaign; remove that JSDoc or move it so
it sits immediately above the sendAnnouncementToUser function. Update the
comment block so deleteAnnouncementCampaign only has its relevant JSDoc
("Permanently remove a campaign...") and sendAnnouncementToUser has the
single-user push JSDoc, ensuring the comment-to-function association matches the
symbols deleteAnnouncementCampaign and sendAnnouncementToUser.
| process.env.AGOO_SMS_API_URL ?? "https://api.agoosms.com/v1/sms/send"; | ||
|
|
||
| const apiKey = process.env.AGOO_SMS_API_KEY; | ||
| const to = process.argv[2] ?? "+233539672914"; |
There was a problem hiding this comment.
Remove hard-coded fallback recipient.
Defaulting to a real phone number can cause accidental outbound sends (cost + privacy risk). Require an explicit to arg (or a dedicated test env var) and fail fast when absent.
Suggested fix
-const to = process.argv[2] ?? "+233539672914";
+const to = process.argv[2] ?? process.env.AGOO_SMS_TEST_TO;
+
+if (!to) {
+ console.error(
+ "Missing recipient. Pass a phone number as argv[2] or set AGOO_SMS_TEST_TO."
+ );
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const to = process.argv[2] ?? "+233539672914"; | |
| const to = process.argv[2] ?? process.env.AGOO_SMS_TEST_TO; | |
| if (!to) { | |
| console.error( | |
| "Missing recipient. Pass a phone number as argv[2] or set AGOO_SMS_TEST_TO." | |
| ); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-dashboard/scripts/test-agoosms.mjs` at line 39, The script currently
falls back to a hard-coded phone number via the const to = process.argv[2] ??
"+233539672914"; which risks accidental sends; change this so the recipient must
be provided explicitly (either via process.argv[2] or a dedicated test env var
like TEST_SMS_TO) and fail fast if neither is set—update the logic around the to
variable in test-agoosms.mjs (the process.argv handling) to check for an
explicit value, throw or exit with a clear error message when missing, and
remove the hard-coded fallback.
| const res = await fetch(API_URL, { | ||
| method: "POST", | ||
| headers: { | ||
| "X-API-Key": apiKey, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ to, message }), | ||
| }); | ||
|
|
||
| const text = await res.text(); | ||
| console.log("HTTP", res.status); | ||
| console.log(text); | ||
| process.exit(res.ok ? 0 : 1); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the file
fd -t f "test-agoosms.mjs" --exec wc -l {}Repository: teebhagg/CediWise
Length of output: 110
🏁 Script executed:
# Read the specific file around lines 51-63
cat -n cediwise-dashboard/scripts/test-agoosms.mjs | head -70Repository: teebhagg/CediWise
Length of output: 2279
🏁 Script executed:
# Check for Node.js version constraints
fd -t f -e "json" -e "nvmrc" -e "npmrc" | xargs grep -l "node\|engines" 2>/dev/null | head -5Repository: teebhagg/CediWise
Length of output: 234
🏁 Script executed:
# Check Node.js version requirements in package.json
cat cediwise-dashboard/package.json | grep -A 5 -B 5 "engines\|node"Repository: teebhagg/CediWise
Length of output: 756
🏁 Script executed:
# Also check if there's a .nvmrc file
fd -name ".nvmrc" -o -name ".node-version" | xargs cat 2>/dev/nullRepository: teebhagg/CediWise
Length of output: 292
🏁 Script executed:
# Check the root package.json or any engines specification
fd -type f -name "package.json" -exec grep -l "engines" {} \;Repository: teebhagg/CediWise
Length of output: 232
🏁 Script executed:
# Look for Node.js version info more directly
cat cediwise-dashboard/package.json | head -20Repository: teebhagg/CediWise
Length of output: 603
🏁 Script executed:
# Verify AbortController availability in Node.js and fetch behavior
node --versionRepository: teebhagg/CediWise
Length of output: 70
🏁 Script executed:
# Check if there are any existing error handling patterns in the codebase for fetch calls
rg "fetch\(" cediwise-dashboard --type js --type mjs -A 5 | head -40Repository: teebhagg/CediWise
Length of output: 89
🏁 Script executed:
# Search for fetch calls in the codebase
rg "fetch\(" cediwise-dashboard -A 5 | head -60Repository: teebhagg/CediWise
Length of output: 2456
🏁 Script executed:
# Check if there are any try/catch patterns around network requests in the project
rg "try\s*{" cediwise-dashboard -A 10 | grep -A 10 "fetch\|request" | head -50Repository: teebhagg/CediWise
Length of output: 139
🏁 Script executed:
# Search for try/catch patterns around network requests
rg "try" cediwise-dashboard -A 15 | grep -B 2 -A 10 "fetch\|response\|error" | head -80Repository: teebhagg/CediWise
Length of output: 5997
🏁 Script executed:
# Check the sms.ts file to see how it handles the fetch error case
cat cediwise-dashboard/lib/actions/sms.ts | head -50Repository: teebhagg/CediWise
Length of output: 1450
Add timeout and explicit fetch error handling.
The request path should be bounded and wrapped; currently network failures can crash noisily or hang indefinitely.
Suggested fix
-const res = await fetch(API_URL, {
- method: "POST",
- headers: {
- "X-API-Key": apiKey,
- "Content-Type": "application/json",
- },
- body: JSON.stringify({ to, message }),
-});
-
-const text = await res.text();
-console.log("HTTP", res.status);
-console.log(text);
-process.exit(res.ok ? 0 : 1);
+const controller = new AbortController();
+const timeout = setTimeout(() => controller.abort(), 15_000);
+
+try {
+ const res = await fetch(API_URL, {
+ method: "POST",
+ headers: {
+ "X-API-Key": apiKey,
+ "Content-Type": "application/json",
+ },
+ body: JSON.stringify({ to, message }),
+ signal: controller.signal,
+ });
+
+ const text = await res.text();
+ console.log("HTTP", res.status);
+ console.log(text);
+ process.exit(res.ok ? 0 : 1);
+} catch (error) {
+ console.error(
+ "AgooSMS request failed:",
+ error instanceof Error ? error.message : String(error)
+ );
+ process.exit(1);
+} finally {
+ clearTimeout(timeout);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const res = await fetch(API_URL, { | |
| method: "POST", | |
| headers: { | |
| "X-API-Key": apiKey, | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify({ to, message }), | |
| }); | |
| const text = await res.text(); | |
| console.log("HTTP", res.status); | |
| console.log(text); | |
| process.exit(res.ok ? 0 : 1); | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 15_000); | |
| try { | |
| const res = await fetch(API_URL, { | |
| method: "POST", | |
| headers: { | |
| "X-API-Key": apiKey, | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify({ to, message }), | |
| signal: controller.signal, | |
| }); | |
| const text = await res.text(); | |
| console.log("HTTP", res.status); | |
| console.log(text); | |
| process.exit(res.ok ? 0 : 1); | |
| } catch (error) { | |
| console.error( | |
| "AgooSMS request failed:", | |
| error instanceof Error ? error.message : String(error) | |
| ); | |
| process.exit(1); | |
| } finally { | |
| clearTimeout(timeout); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-dashboard/scripts/test-agoosms.mjs` around lines 51 - 63, Wrap the
fetch call in an AbortController timeout and a try/catch to handle network
errors and prevent hangs: create an AbortController, start a setTimeout to call
controller.abort() after a chosen timeout (e.g., 10s), pass controller.signal to
fetch(API_URL, { ..., signal }), then await fetch inside try/catch; on success
read res.text(), clear the timeout, log status and body, and exit 0 for res.ok
or 1 otherwise; on catch log the error (including abort/errors) and exit
non‑zero. Reference symbols: API_URL, apiKey, to, message, fetch call returning
res, res.text(), and process.exit.
| ``` | ||
| CediWise/ | ||
| ├── assets/ # Shared images (banner, logo) | ||
| ├── cediwise-mobile-app/ # Expo React Native app (Android, iOS) | ||
| ├── supabase/ # Canonical Supabase CLI root (migrations, Edge Functions, seeds) | ||
| ├── scripts/ # Repo-level ops (backup, sync baseline, seeds — see each script) | ||
| ├── legacy/ # Archived historical SQL (read-only; do not re-apply) | ||
| ├── cediwise-mobile-app/ # Expo React Native app (Android, iOS) — EAS, app.json, eas.json | ||
| │ ├── app/ # Expo Router screens | ||
| │ ├── components/ # UI components | ||
| │ ├── calculators/ # PAYE/SSNIT, T-Bill, savings, loan, cash flow | ||
| │ ├── content/ # Bundled lesson content | ||
| │ └── supabase/ # Schema, migrations (FLM, analytics) | ||
| │ ├── components/ | ||
| │ ├── calculators/ | ||
| │ ├── distribution/ # Per-version release notes (*.txt) for CI — required for store deploys | ||
| │ └── content/ # Bundled lesson content (app) | ||
| ├── cediwise-dashboard/ # Next.js admin dashboard | ||
| │ ├── app/(dashboard)/ # Users, learning-data, feedback, emails, announcements | ||
| │ └── supabase-functions/ # Edge functions (e.g. send-email-campaign) | ||
| ├── cediwise-web-official/ # TanStack Start web app (landing, legal, feature pages) | ||
| ├── content/ # Lesson content (Markdown) | ||
| ├── docs/ # Specs, plans, PRDs | ||
| │ ├── app/(dashboard)/ | ||
| │ └── scripts/ # e.g. SMS provider tests | ||
| ├── cediwise-web-official/ # TanStack Start + Vite (marketing site; Vercel) | ||
| ├── content/ # Lesson source content (Markdown), shared with tooling | ||
| ├── docs/ # Specs, plans, PRDs (root-level docs) | ||
| ├── .github/workflows/ # CI: mobile PR checks; Android & iOS EAS build + submit | ||
| ├── LICENSE | ||
| └── .github/ # CI/CD (Android builds) | ||
| ├── CONTRIBUTING.md | ||
| └── README.md # This file | ||
| ``` |
There was a problem hiding this comment.
Specify a language for the fenced project-structure block.
Line 70 opens a fenced block without a language, triggering markdownlint MD040.
Proposed fix
-```
+```text
CediWise/
├── assets/ # Shared images (banner, logo)
...
└── README.md # This file
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 70 - 92, The fenced project-structure block in
README.md is missing a language tag and triggers markdownlint MD040; update the
opening fence for that tree block (the triple-backtick block that begins the
CediWise/ project tree) to include a language identifier such as "text" (i.e.,
change ``` to ```text) so the code fence is explicitly typed and the linter
warning is resolved.
| [functions.subscription-janitor] | ||
| verify_jwt = false | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Config check =="
rg -n -C2 '^\[functions\.subscription-janitor\]|verify_jwt\s*=' supabase/config.toml
echo
echo "== Locate subscription-janitor function files =="
JANITOR_DIR="$(fd -i '^subscription-janitor$' supabase/functions -t d | head -n1 || true)"
if [ -z "${JANITOR_DIR}" ]; then
echo "No subscription-janitor directory found under supabase/functions"
exit 0
fi
echo "Found: ${JANITOR_DIR}"
echo
echo "== Inspect auth/secret checks in function code =="
fd -e ts -e js . "${JANITOR_DIR}" | while read -r f; do
echo "---- ${f} ----"
rg -n -C2 'Authorization|Bearer|verify_jwt|x-cron|x-secret|CRON|ENABLE_AUTO_DOWNGRADE|SERVICE_ROLE|Deno\.env\.get' "${f}" || true
doneRepository: teebhagg/CediWise
Length of output: 1404
subscription-janitor is currently public (verify_jwt = false), exposing billing operations.
For a function that performs subscription downgrades, disabling JWT verification is a security risk. While the function includes an ENABLE_AUTO_DOWNGRADE environment check, this is a feature flag—not authentication. An attacker can still invoke the endpoint repeatedly.
Set verify_jwt = true to enforce proper request-level authentication via JWT, or implement a strong shared secret check inside the function if public invocation is intentional.
Suggested config hardening
[functions.subscription-janitor]
-verify_jwt = false
+verify_jwt = true📝 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.
| [functions.subscription-janitor] | |
| verify_jwt = false | |
| [functions.subscription-janitor] | |
| verify_jwt = true | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/config.toml` around lines 378 - 380, The subscription-janitor
function is exposed because verify_jwt is false; change the function
configuration for subscription-janitor to require authentication by setting
verify_jwt = true (or if public invocation is intentional, implement a robust
shared-secret check inside the subscription-janitor handler) and keep the
existing ENABLE_AUTO_DOWNGRADE feature flag only as a feature toggle (not as
authentication); ensure the function's code verifies the JWT (or validates the
shared secret) before performing any billing/subscription downgrade operations.
…d enhanced error handling
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ac1f54d98
ℹ️ 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".
| ALTER TABLE "public"."subscriptions" ENABLE ROW LEVEL SECURITY; | ||
|
|
||
|
|
||
| CREATE POLICY "subscriptions_self" ON "public"."subscriptions" USING (("auth"."uid"() = "user_id")) WITH CHECK (("auth"."uid"() = "user_id")); |
There was a problem hiding this comment.
The migration gives authenticated users direct write access to their own subscriptions row (subscriptions_self with WITH CHECK (auth.uid() = user_id)), while this table stores entitlement-critical fields (plan, status, billing dates, etc.). In practice, any signed-in client can issue a direct update/insert and self-upgrade to paid tiers or alter billing state without a verified payment webhook, which breaks billing integrity and paid-feature access control.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cediwise-mobile-app/app/upgrade.tsx`:
- Around line 1265-1273: The code currently falls back to an empty string for
process.env.EXPO_PUBLIC_PAYSTACK_PUBLIC_KEY and mounts PaystackProvider with an
unusable key; change this to fail fast by validating the environment variable
before rendering: check the EXPO_PUBLIC_PAYSTACK_PUBLIC_KEY into publicKey and
if it is falsy, do not render PaystackProvider (and instead surface an error
state or throw) so paid-plan checkout is blocked; update the logic around the
publicKey variable and the PaystackProvider/UpgradeScreenContent render path so
the app either displays a clear error message or prevents mounting
PaystackProvider when the key is missing.
In `@cediwise-mobile-app/hooks/usePeriodicFeedbackPrompt.ts`:
- Around line 191-203: The cleanup inside useFocusEffect is calling
persistDismiss() which triggers the 21-day cooldown unintentionally whenever
dependencies change; remove the persistDismiss() invocation from the cleanup
returned by useFocusEffect (leave clearShowTimer(), openedThisFocusRef.current =
false, modalVisibleRef handling and setModalVisible(false) intact) and ensure
persistDismiss() is only called from explicit user interaction handlers (onLater
and onNeverAsk) where userId is available; update onLater/onNeverAsk to call
persistDismiss() (with the same userId checks) and remove any other
cleanup-triggered persistence paths.
- Around line 55-60: The quick-submit path uses setWorking(true) which only
prevents duplicates after a render; add a same-tick in-flight ref guard (e.g.,
submittingRef = useRef(false)) and check it at the start of submitQuickRating to
return early if already true, set submittingRef.current = true immediately
before initiating the async submit, and clear it (and setWorking(false)) in
finally/error branches so rapid taps cannot call submitQuickRating twice;
reference submitQuickRating, setWorking, and existing refs (showTimerRef,
openedThisFocusRef, modalVisibleRef) to locate where to add the guard.
- Around line 92-179: evaluateAndMaybeSchedule can resume after async storage
loads and fire the timer on the wrong screen; add an in-flight guard by
generating a runId (or AbortController) at the top of evaluateAndMaybeSchedule,
store it on a ref (e.g., currentRunIdRef), and before any async continuations
and before setTimeout callback check that the runId still matches (or that
!aborted) and abort/return if not; also clear showTimerRef and cancel any
pending runId/abort in the useFocusEffect cleanup so stale runs are invalidated
on blur/teardown. Stop the cleanup from calling persistDismiss on
dependency-driven re-runs by tracking real focus state with a focusActiveRef set
on focus/blur (used by useFocusEffect) and only call persistDismiss when the
screen actually blurred/unmounted (i.e., focusActiveRef transitions false), not
on any cleanup caused by dependency change. Finally, add a same-tick reentrancy
guard to submitQuickRating using a ref-based boolean (like feedback.tsx) to
short-circuit if an insert is already in-flight instead of relying solely on the
working state.
In `@cediwise-mobile-app/utils/smeExport.ts`:
- Line 79: Replace the LF-only join with a CRLF join to produce
RFC4180-compatible CSV rows: change the return that currently uses
lines.join("\n") to use lines.join("\r\n") (adjusting the CSV-producing function
that constructs the lines array and the return statement referencing lines.join)
so rows are delimited with "\r\n" for strict CSV consumers.
- Around line 10-13: The csvCell function's formula neutralization only checks
the very first character (via /^[=+\-@]/) and can be bypassed by leading
whitespace or control characters; update the detection regex in csvCell to match
optional leading whitespace and control characters before the formula trigger
(for example use something like /^\s*[\u0000-\u001F]*[=+\-@]/) so that strings
with leading spaces or control chars are also neutralized, keeping the existing
neutralization behavior (prefixing with a single quote) when the expanded test
matches.
In `@cediwise-mobile-app/utils/subscriptionReconciliation.ts`:
- Around line 53-67: The helper fetchSubscriptionRow currently masks Supabase
read failures by returning null on error, which makes backend errors
indistinguishable from "no row yet" for waitForSubscriptionReconciliation;
change fetchSubscriptionRow to throw the Supabase error (or a wrapped Error with
context) when the query returns an error instead of returning null, preserving
the existing maybeSingle behavior so a genuinely missing row still yields null
(SubscriptionReconcileRow | null); update the error message to include context
like the userId and the original error.message so callers such as
waitForSubscriptionReconciliation can detect and abort on terminal read
failures.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0da25546-352e-45b5-a9f3-bde3a360c86a
📒 Files selected for processing (10)
cediwise-mobile-app/app/feedback.tsxcediwise-mobile-app/app/profile/edit.tsxcediwise-mobile-app/app/upgrade.tsxcediwise-mobile-app/components/features/budget/BudgetEngineModeList.tsxcediwise-mobile-app/constants/feedback.tscediwise-mobile-app/distribution/0.2.6.txtcediwise-mobile-app/hooks/usePeriodicFeedbackPrompt.tscediwise-mobile-app/utils/smeExport.tscediwise-mobile-app/utils/subscriptionReconciliation.tscediwise-mobile-app/utils/subscriptionSync.ts
| const publicKey = process.env.EXPO_PUBLIC_PAYSTACK_PUBLIC_KEY || ""; | ||
|
|
||
| return ( | ||
| <PaystackProvider | ||
| publicKey={publicKey} | ||
| currency="GHS" | ||
| defaultChannels={payMethod === "momo" ? ["mobile_money"] : ["card"]} | ||
| > | ||
| <UpgradeScreenContent payMethod={payMethod} setPayMethod={setPayMethod} /> |
There was a problem hiding this comment.
Fail fast when the Paystack public key is missing.
Falling back to "" hides a deployment misconfiguration until the user taps pay. Guard this case and block paid-plan checkout with a clear error instead of mounting PaystackProvider with an unusable key.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-mobile-app/app/upgrade.tsx` around lines 1265 - 1273, The code
currently falls back to an empty string for
process.env.EXPO_PUBLIC_PAYSTACK_PUBLIC_KEY and mounts PaystackProvider with an
unusable key; change this to fail fast by validating the environment variable
before rendering: check the EXPO_PUBLIC_PAYSTACK_PUBLIC_KEY into publicKey and
if it is falsy, do not render PaystackProvider (and instead surface an error
state or throw) so paid-plan checkout is blocked; update the logic around the
publicKey variable and the PaystackProvider/UpgradeScreenContent render path so
the app either displays a clear error message or prevents mounting
PaystackProvider when the key is missing.
| function csvCell(value: string | number | boolean): string { | ||
| const raw = String(value); | ||
| const neutralized = /^[=+\-@]/.test(raw) ? `'${raw}` : raw; | ||
| if (/[",\n\r]/.test(neutralized)) { |
There was a problem hiding this comment.
Harden formula neutralization against leading whitespace/control-char bypasses.
On Line 12, the check only matches when the first character is =, +, -, or @. Spreadsheet formula payloads can be prefixed with whitespace/control chars and still execute in some clients. Expand the guard to detect formula triggers after optional leading whitespace.
Suggested fix
function csvCell(value: string | number | boolean): string {
const raw = String(value);
- const neutralized = /^[=+\-@]/.test(raw) ? `'${raw}` : raw;
+ const neutralized = /^\s*[=+\-@]/.test(raw) ? `'${raw}` : raw;
if (/[",\n\r]/.test(neutralized)) {
return `"${neutralized.replace(/"/g, '""')}"`;
}
return neutralized;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-mobile-app/utils/smeExport.ts` around lines 10 - 13, The csvCell
function's formula neutralization only checks the very first character (via
/^[=+\-@]/) and can be bypassed by leading whitespace or control characters;
update the detection regex in csvCell to match optional leading whitespace and
control characters before the formula trigger (for example use something like
/^\s*[\u0000-\u001F]*[=+\-@]/) so that strings with leading spaces or control
chars are also neutralized, keeping the existing neutralization behavior
(prefixing with a single quote) when the expanded test matches.
| lines.push(row.map(csvCell).join(",")); | ||
| } | ||
|
|
||
| return lines.join("\n"); |
There was a problem hiding this comment.
Use CRLF row separators for strict CSV compatibility.
On Line 79, joining with "\n" can cause interoperability issues with strict RFC4180 consumers. Prefer "\r\n" row delimiters.
Suggested fix
- return lines.join("\n");
+ return lines.join("\r\n");📝 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.
| return lines.join("\n"); | |
| return lines.join("\r\n"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cediwise-mobile-app/utils/smeExport.ts` at line 79, Replace the LF-only join
with a CRLF join to produce RFC4180-compatible CSV rows: change the return that
currently uses lines.join("\n") to use lines.join("\r\n") (adjusting the
CSV-producing function that constructs the lines array and the return statement
referencing lines.join) so rows are delimited with "\r\n" for strict CSV
consumers.
… and error handling
|
❌ TypeScript errors ( |
There was a problem hiding this comment.
💡 Codex Review
CediWise/cediwise-mobile-app/app/upgrade.tsx
Lines 582 to 586 in 448e117
Cancel gateway subscription when scheduling downgrade
This downgrade path only updates local subscriptions fields (pending_tier, pending_tier_start_date, cancel_at_period_end) and never triggers a server-side Paystack non-renew/disable action, so recurring Paystack subscriptions can continue charging even after the app reports a scheduled downgrade. In production this creates a billing mismatch for users on auto-renewing plans, because the database intent changes but the payment gateway schedule does not.
ℹ️ 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".
| status: (newTier === "free") ? "expired" : "active", | ||
| cancel_at_period_end: false, | ||
| pending_tier: null, | ||
| pending_tier_start_date: null, | ||
| next_billing_date: null, |
There was a problem hiding this comment.
When handling subscription.disable with a paid pending_tier (for example, downgrading SME to budget), this update sets status to active but also nulls next_billing_date. That leaves an active paid row without a billing anchor, and downstream maintenance logic (e.g., subscription-janitor querying only rows where next_billing_date is not null) will skip renewal/grace processing for that subscription.
Useful? React with 👍 / 👎.
1 participant
Summary
This branch moves Supabase configuration, migrations, and selected Edge Functions to a repository-level
supabase/workspace, archives historical SQL underlegacy/, and continues subscription / billing v2 work (migrations plus Paystack-related functions). In parallel it updates CediWise mobile (tier/auth/supabase plumbing, SME transactions work, notifications/feedback UX and supporting hooks/stores/utils) and the dashboard (announcements, feedback, notifications pages and server actions), with smaller doc and marketing-site touches.Monorepo & database
supabase/with config, generated schema, migrations (including remote baseline, feedback/announcement reads, subscription billing v2, webhook dedupe / realtime / auto-downgrade, proration helpers, feedback constraint fix), seeds, and functions (Paystack initiate/webhook, send-announcement, moved daily-cash-flow-check and delete-account).legacy/inventory README and consolidate prior dashboard/mobile/web migrations and loose Supabase artifacts for traceability without mixing them into the new migration line..gitignore,README.md, andbackups/README.mdupdates.Mobile app (
cediwise-mobile-app)TierContext,tierGate, auth/supabase helpers, EAS/build script tweaks; design docs for subscription billing redesign (v1/v2).(sme)/transactions.tsxand(tabs)/index.tsx; budget settings tweaks;upgrade.tsxaligned with monetization flows.notifications.tsxwithapp/notifications/stack (_layout,index,inbox), plusnotificationsStoreandservices/notificationschanges.app/feedback.tsx,PeriodicFeedbackPromptModal,usePeriodicFeedbackPrompt,feedbackPromptStorage;useSMETransactionFilters,smeExport,subscriptionSync/subscriptionReconciliation,constants/billingPlans,BudgetEngineModeList.app/profile/edit.tsxand related layout/index changes.Dashboard (
cediwise-dashboard)announcement-delete-button(untracked);announcementsandfeedbackactions extended; notifications page refresh.Web & misc
cediwise-web-officialHero tweaks.scripts/backup/sync tooling,supabase/docs/, additional Edge Functions (billing-schedule-cadence,billing-upgrade-quote,subscription-janitor,paystack-momo-*,_shared),cediwise-dashboard/scripts/,content/, and design/plan markdown underdocs/plans/. Avoid committing.DS_Storeunless your team wants it.Reviewer notes
(1)rootsupabase+ migrations + legacy,(2)mobile tier/billing/notifications,(3)dashboard announcements/feedback/notifications).Checklist
.DS_Store/ verify.gitignore