Skip to content

feat(accounts): capability state machine + headline status nomenclature (ENG-516)#452

Open
islandbitcoin wants to merge 3 commits into
mainfrom
eng-516/account-capability-state-machine
Open

feat(accounts): capability state machine + headline status nomenclature (ENG-516)#452
islandbitcoin wants to merge 3 commits into
mainfrom
eng-516/account-capability-state-machine

Conversation

@islandbitcoin

@islandbitcoin islandbitcoin commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Backend half of ENG-516 (Account Upgrade Revamp). Retires Pro and International as user-facing tiers, renames Merchant → Business, and models the account as a set of capability flags from which the internal level is derived. Companion frappe-flash-admin PR adopts the new names in the admin panel; mobile presentation lands with ENG-513.

Product decision (2026-07-14): light headline status — the account leads with one word (Trial → Verified → Business) with capability badges as supporting detail.

The state machine

Account = { verified, bankPayout, business, usdAccount }

verified (phone + ID)                    → L1
+ bankPayout, individual                 → L2  (business-less Pro — ENG-515)
+ business (name + address) + bankPayout → L3
usdAccount (Bridge KYC)                  → orthogonal flag, any level ≥ L1
  • GraphQL capabilities/statusHeadline resolve from one memoized derivation per account per request (WeakMap keyed on the account object) — one ERPNext lookup no matter how many fields ask; the Bridge-KYC "approved" literal is a typed domain constant (BRIDGE_KYC_APPROVED).
  • src/domain/accounts/capabilities.ts — pure derivation: deriveLevelFromCapabilities, deriveStatusHeadline, deriveCapabilitiesForAccount (read model over stored level + ERPNext bank accounts + bridgeKycStatus, with grandfathering for existing L2/L3 accounts).
  • GraphQL: capabilities: AccountCapabilities! and statusHeadline: AccountStatusHeadline! on ConsumerAccount (public) and AuditedAccount (admin). level stays exposed but documented as internal.
  • Capability transitions: new accountCapabilityUpgradeRequest mutation — the client asks for one capability (BANK_PAYOUT or BUSINESS); the target level is derived server-side and the request flows down the existing ERPNext Account Upgrade Request pipeline unchanged. businessAccountUpgradeRequest (whole-tier) remains for existing clients.
  • Full spec: docs/account-capabilities.md.

Testing

  • TEST="capabilities|request-capability-upgrade|get-account-capabilities" yarn test:unit — 27 tests: derivation matrix, headline mapping, grandfathering, round-trip; requestCapabilityUpgrade routing (unverified / already-has / missing-bank / L2 add-bank / L3 reuse-on-file); getAccountCapabilities ERPNext-failure fallback, no-erpParty skip, and memoization semantics.
  • Full unit suite: 135 suites / 1064 passed.
  • yarn tsc-check clean; SDL regenerated via yarn write-sdl (public + admin + supergraph).

Local testing

query { me { defaultAccount { ... on ConsumerAccount { statusHeadline capabilities { verified bankPayout business usdAccount } } } } }

mutation { accountCapabilityUpgradeRequest(input: {
  capability: BANK_PAYOUT
  fullName: "Test User"
  address: { title: "Home", line1: "1 Main St", city: "Kingston", state: "St Andrew", country: "Jamaica" }
  bankAccount: { bankName: "NCB", bankBranch: "Half Way Tree", accountType: "Savings", currency: "JMD", accountNumber: "123456789" }
}) { id status errors { message } } }

🤖 Generated with Claude Code

…re (ENG-516)

Retire Pro/International as user-facing tiers; rename Merchant to Business.
Model the account as capability flags {verified, bankPayout, business,
usdAccount} and derive the internal level (L1-L3) from them. Expose
capabilities + statusHeadline (light headline status: Trial / Verified /
Business) on ConsumerAccount and AuditedAccount, and add the
accountCapabilityUpgradeRequest mutation so clients request a single
capability instead of a whole tier. Spec in docs/account-capabilities.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear

linear Bot commented Jul 15, 2026

Copy link
Copy Markdown

ENG-516

@islandbitcoin

Copy link
Copy Markdown
Contributor Author

Review — capability state machine (ENG-516)

Domain derivation module + the 15-test matrix are solid. Four items to action before merge:

1. 🔴 CI is red — new mutation not registered in scope-map.ts.
api-key-scope-enforcement.spec.ts › "maps every authed root field" fails: accountCapabilityUpgradeRequest is unmapped (deny-by-default net). Add accountCapabilityUpgradeRequest: "BLOCKED" to src/domain/api-keys/scope-map.ts (matches its sibling businessAccountUpgradeRequest). The full yarn test:unit is red — the scoped TEST=capabilities run hid it; run the full suite before green-claiming.

2. 🟠 An unverified (L0) account can mint an L2 upgrade request.
request-capability-upgrade.ts computes level = deriveLevelFromCapabilities(...) — correctly L0 when !verified — but only special-cases === L3; L0/L1/L2 all fall through to an L2 request. The only downstream guard, AccountUpgradeRequest.validate() (AccountUpgradeRequest.ts:16), is requestedLevel <= currentLevel → reject, so 2 > 0 passes. Net: a phone-only, un-ID'd account can request bank-payout → L2, skipping the verified prerequisite the state machine is built on. Add if (!targetCapabilities.verified) return new ValidationError(...) before creating the request. (Pre-existing at the validate layer, but this handler is the one that derives L0 and then ignores it.)

3. 🟡 input.bankAccount as BankAccount (request-capability-upgrade.ts:78) casts away undefined.
On the business-with-on-file-bank path input.bankAccount is undefined; the cast satisfies createUpgradeRequest's L3 type (bankAccount required) but sends empty bank fields to ERPNext. No crash (validate() uses this.bankAccount?.…), but a grandfathered account whose ERPNext bank is missing/null → an incomplete L3 request. Make the L3 bankAccount optional in the type (honest) or confirm a real bank exists before deriving business.

4. 🟡 requestCapabilityUpgrade has no tests.
The 15 tests cover the pure functions; the mutation orchestration — where #2 and #3 live — is untested. Add: unverified → rejected, business with on-file bank (no bankAccount submitted), and already-has-capability.

Minor: the read-model derived level can exceed the still-exposed stored level (an L1 account with a bank on file reads bankPayout: true → implies L2). Known per your "consistent accounts" round-trip test; worth a doc note since both fields ship over GraphQL.

Blocking: #1 and #2.

bobodread876 and others added 2 commits July 15, 2026 22:39
…unt typing (ENG-516)

Review follow-ups on the capability state machine:

- Register accountCapabilityUpgradeRequest in the API-key scope map (BLOCKED,
  like businessAccountUpgradeRequest). The deny-by-default completeness test
  was red because the new mutation was unmapped.
- requestCapabilityUpgrade now rejects unverified accounts: the state machine
  derives L0 for !verified, so a request from one would otherwise become a
  level-skipping L2 ERPNext request (validate() only guards requested<=current).
- Type the L3 upgrade's bankAccount as optional and drop the `as BankAccount`
  cast — a business upgrade legitimately reuses a bank already on file, and
  validate() already reads it via optional chaining.
- Add unit tests for requestCapabilityUpgrade (unverified, already-has,
  missing-bank, L2 add-bank, L3 reuse-on-file).
- Doc note on derived-vs-stored level divergence.

Verified: tsc --noEmit clean; full unit suite 134 suites / 1057 passed
(api-key-scope-enforcement now green).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eral (ENG-516)

Review follow-ups:

- getAccountCapabilities memoizes per account object (WeakMap): the
  capabilities and statusHeadline GraphQL fields each resolved it
  separately, costing two serial ERPNext round-trips per query — and
  N×2 on admin views that list accounts. One lookup now serves every
  field resolution for the request; no cross-request staleness since
  each request fetches a fresh account object.
- BRIDGE_KYC_APPROVED constant typed against Account["bridgeKycStatus"]
  replaces the bare "approved" magic string in the derivation, so an
  upstream status rename is a compile error instead of a silently-false
  usdAccount.
- getAccountCapabilities unit tests: ERPNext-failure fallback (the
  branch that runs during an ERPNext incident), no-erpParty skip,
  bank-on-file, memoization semantics.
- requestCapabilityUpgrade return annotation was Promise<Promise<...>>
  via ReturnType of an async fn; unwrapped.

tsc clean; full unit suite 135 suites / 1064 passed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

3 participants