Skip to content

fix(kyc): Dr Green decides verification, not a local flag - #223

Merged
AutomatosAI merged 1 commit into
mainfrom
fix/kyc-drgreen-source-of-truth
Jul 29, 2026
Merged

fix(kyc): Dr Green decides verification, not a local flag#223
AutomatosAI merged 1 commit into
mainfrom
fix/kyc-drgreen-source-of-truth

Conversation

@AutomatosAI

@AutomatosAI AutomatosAI commented Jul 29, 2026

Copy link
Copy Markdown
Owner

The bug, from today

checkUserKycStatus short-circuited on BudStacks' own consultation_questionnaires.isKycVerified column:

// If explicitly verified in local DB, trust it
if (questionnaire?.isKycVerified) {
    return { isLoggedIn: true, kycVerified: true, status: "ACTIVE" };
}

It returned ACTIVE without ever calling Dr Green. And the persist step below only ever wrote true, never false — so the column was a one-way latch. Once set, the API was never consulted again and the two systems could drift apart permanently.

Production, 2026-07-29: a client saw "You're verified — your account is approved — start shopping" from that latch, while Dr Green production had no such client at all. Nothing surfaced it until checkout, where the order died with a 500:

Could not retrieve clientCartId from Dr Green for client 83b36bd8-…
Client record could not be found via /dapp/clients/{id} or list scan.

Same account, minutes apart. The green badge and the hard failure were both "correct" according to the system that produced them.

Change

Verification is answered by the system that owns it, at the moment it is asked.

  • The local flag no longer short-circuits. Dr Green is called every time.
  • The local row becomes a mirror, written in BOTH directions, so drift corrects itself. It is kept only because /api/consultation/status and components/shop/RestrictedRegionGate still read it — it carries no authority here.
  • The orphan-tenant migration path no longer hardcodes isKycVerified: true, which would have re-created the latch for any user whose questionnaire row lives under a different tenant.
  • REJECTED is no longer an early return before the mirror — a client verified once and later rejected kept reading as verified on every other surface.
  • Unreachable API still fails closed as API_ERROR, which the dashboard already renders distinctly (rose, different copy) from "not verified". A real customer during an outage is told we can't check right now, not that they're unverified.

Expected visible effect

Anyone currently holding a stale isKycVerified = true with no matching Dr Green client will flip from verified to unverified on their next dashboard load. That is the correct answer — they could not have completed an order anyway — but it is customer-visible, so it's worth knowing the count first:

SELECT count(*) FROM consultation_questionnaires WHERE "isKycVerified" = true;

against how many of those users have a drGreenClientId prod still recognises.

Tests

tests/unit/kyc-check-source-of-truth.test.ts — local true + Dr Green false (API is called, Dr Green wins); false mirrored back; the verified path still works; REJECTED clears the flag; fail-closed on API error.

Not in this PR

  • getClientById on the backend throws a bare Error for a missing client, so it returns 500 instead of 404. That's why BudStacks can't distinguish "absent" from "broken" and burns a 40-page bidirectional scan on every miss.
  • RestrictedRegionGate reads the mirrored column directly rather than asking Dr Green. Now self-correcting, but still second-hand.

Summary by CodeRabbit

  • Bug Fixes

    • KYC verification now reflects the latest status from the external verification service.
    • Verification changes, including rejection or loss of verification, are synchronized correctly.
    • Rejected applications now return the rejection reason and document status.
    • If verification services are unavailable, the system reports an error instead of relying on outdated local data.
  • Tests

    • Added coverage for verification updates, rejections, and service failures.

checkUserKycStatus short-circuited on the local
consultation_questionnaires.isKycVerified column and returned ACTIVE
without ever calling Dr Green. The persist step below it only ever wrote
`true`, never `false`, so the column was a one-way latch: once set, the
API was never consulted again and the two systems could drift apart
permanently.

Observed in production 2026-07-29: a client saw "You're verified — start
shopping" from that latch while Dr Green production had no such client at
all. Nothing surfaced it until checkout, where the order died with a 500
("Could not retrieve clientCartId... Client record could not be found").
The green badge and the failed order were the same account, minutes apart.

Verification state is answered by the system that owns it, at the moment
it is asked.

- the local flag no longer short-circuits; Dr Green is called every time
- the local row is now a MIRROR written in BOTH directions, so drift
  corrects itself. It is kept only because /api/consultation/status and
  components/shop/RestrictedRegionGate still read it — it carries no
  authority here.
- the orphan-tenant migration path no longer hardcodes isKycVerified:true,
  which would have re-created the latch for users whose questionnaire row
  lives under another tenant
- REJECTED is no longer an early return before the mirror, so a client who
  was verified once and later rejected stops reading as verified elsewhere
- unreachable API still fails CLOSED as status API_ERROR, which the
  dashboard already renders distinctly from "not verified"

Tests cover: local true + Dr Green false, false mirrored back, verified
path, REJECTED clearing the flag, and fail-closed on API error.
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 58aa7721-1798-461b-a69c-74ed1e1e8e5f

📥 Commits

Reviewing files that changed from the base of the PR and between 105d683 and 3be119d.

📒 Files selected for processing (2)
  • nextjs_space/app/actions/kyc-check.ts
  • nextjs_space/tests/unit/kyc-check-source-of-truth.test.ts

📝 Walkthrough

Walkthrough

checkUserKycStatus now treats Dr Green as the runtime KYC authority, mirrors both verification outcomes to the current tenant’s questionnaire row, and processes rejected approvals after synchronization. New Vitest tests cover these paths and API failures.

Changes

KYC source-of-truth synchronization

Layer / File(s) Summary
Dr Green status flow
nextjs_space/app/actions/kyc-check.ts, nextjs_space/tests/unit/kyc-check-source-of-truth.test.ts
Removes the local verified short-circuit and returns API_ERROR without falling back to the local flag when Dr Green is unavailable.
Questionnaire mirror and rejection handling
nextjs_space/app/actions/kyc-check.ts
Mirrors Dr Green’s true or false status, migrates questionnaire rows to the current tenant when needed, and returns rejected responses after mirroring.
KYC behavior validation
nextjs_space/tests/unit/kyc-check-source-of-truth.test.ts
Tests Dr Green lookups, local synchronization, active responses, rejection responses, and API failures.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: gerard161-site

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/kyc-drgreen-source-of-truth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AutomatosAI
AutomatosAI merged commit e8b0b02 into main Jul 29, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants