Context
Follow-up from review of #205 (feat(core): add the non-custodial wallet partner user role).
Finding
packages/core/src/__tests__/user-role.test.ts pins three invariants over Object.values(UserRole):
- no duplicate values (
user-role.test.ts:6-15)
- anchored PascalCase
/^[A-Z][A-Za-z]*$/ (user-role.test.ts:18-24)
- fixed member count 13 (
user-role.test.ts:26-28)
These do not pin the exact contract string for the new member in packages/core/src/definitions/jwt.ts:16:
NON_CUSTODIAL_WALLET_PARTNER = 'NonCustodialWalletPartner',
A typo such as 'NonCustodialWalletPartnr' would still satisfy uniqueness, regex, and length. The PR body intentionally rejected a tautological one-liner and preferred invariants — that is a valid design choice, not a merge-blocker for #205.
Separately, a fixed toHaveLength(13) fails on any parallel role addition (e.g. historical PARTNER work on other branches) and forces mechanical updates that do not improve contract safety on their own.
Evidence
Suggested fix
Either:
- add an explicit pin for the new member:
expect(UserRole.NON_CUSTODIAL_WALLET_PARTNER).toBe('NonCustodialWalletPartner')
(and keep or drop the length check), or
- pin the full expected value list and update that list deliberately when roles change.
Not a merge-blocker for #205: production behaviour of the additive enum member is correct; this is test strength only.
Related
Context
Follow-up from review of #205 (
feat(core): add the non-custodial wallet partner user role).Finding
packages/core/src/__tests__/user-role.test.tspins three invariants overObject.values(UserRole):user-role.test.ts:6-15)/^[A-Z][A-Za-z]*$/(user-role.test.ts:18-24)user-role.test.ts:26-28)These do not pin the exact contract string for the new member in
packages/core/src/definitions/jwt.ts:16:A typo such as
'NonCustodialWalletPartnr'would still satisfy uniqueness, regex, and length. The PR body intentionally rejected a tautological one-liner and preferred invariants — that is a valid design choice, not a merge-blocker for #205.Separately, a fixed
toHaveLength(13)fails on any parallel role addition (e.g. historicalPARTNERwork on other branches) and forces mechanical updates that do not improve contract safety on their own.Evidence
packages/core/src/__tests__/user-role.test.ts:6-28packages/core/src/definitions/jwt.ts:16Suggested fix
Either:
expect(UserRole.NON_CUSTODIAL_WALLET_PARTNER).toBe('NonCustodialWalletPartner')(and keep or drop the length check), or
Not a merge-blocker for #205: production behaviour of the additive enum member is correct; this is test strength only.
Related