MemberJunction / MemberJunction/MJ

The newUserRoles default is duplicated in three files with nothing keeping them in sync

Open Beginner friendly
#4,285 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

**Found by** the PR gauntlet on #4275 (`fix/4260-new-user-roles-default`), 2026-09-07. Structural gap, pre-existing at `f2ec156daf31670b42d7704e994686b09daf14df`; that PR had to correct all three files by hand, which is what surfaced it.

### Where
`packages/MJServer/src/config.ts:676` — the shipped `DEFAULT_SERVER_CONFIG`
`packages/MJServer/README.md:131` — the documented example config
`packages/MJCLI/mj.config.cjs:230` — the in-repo reference config

### What happens
All three currently say `newUserRoles: ['UI']`. Change any one of them and nothing fails: no test, no lint rule, no CI gate compares them. The existing regression test `packages/MJServer/src/__tests__/newUserRolesDefault.test.ts` pins only `DEFAULT_SERVER_CONFIG`.

### Why it matters
This is a security-relevant default. It governs what every auto-provisioned identity is granted on a deployment that does not name `newUserRoles` explicitly — and auto-provisioning is on by default with no domain restriction. The two files the test does *not* cover are precisely the ones operators copy into real deployments, so a drift there ships a wrong default to hosts while the test stays green. Issue #4260 was this default being wrong; the cost of it being wrong again is the same.

### Repro
```bash
sed -i '' "s/newUserRoles: \['UI'\]/newUserRoles: ['UI', 'Developer']/" packages/MJServer/README.md
cd ~/Projects/mj-dev && pnpm --filter @memberjunction/server run test # passes
cd ~/Projects/mj-dev/MJ && pnpm run check:standards # passes
```

### Evidence
```
packages/MJServer/src/config.ts:676 newUserRoles: ['UI'],
packages/MJServer/README.md:131 newUserRoles: ['UI'],
packages/MJCLI/mj.config.cjs:230 newUserRoles: ['UI'],
```
Three copies, one test, covering one of them.

### Suggested fix
Extend `newUserRolesDefault.test.ts` with a case that reads both template files from disk, extracts the `newUserRoles` array, and asserts each equals `DEFAULT_SERVER_CONFIG.userHandling.newUserRoles`. A test is the right place rather than a `check:` script: it lives beside the existing contract test for this same value, runs in the normal unit tier, and fails with a message pointing at the value that drifted. Keep the extraction deliberately simple (a regex for the single line) and let it fail loudly if the shape changes — a brittle-but-loud check is correct here, since a silent skip would restore exactly the gap being closed.

### Definition of done
- [ ] A failing test that reproduces it (edit one template, watch it fail), then green
- [ ] All three locations are asserted equal by one test
- [ ] Existing suite and gates green; no changed expectation in an existing test

### Verify by
Running the repro above: `pnpm --filter @memberjunction/server run test` now FAILS, naming the drifted file.

Contributor guide

Open the contributing guide

Research direction

Start with packages/MJServer/src/__tests__/newUserRolesDefault.test.ts and inspect how it currently checks DEFAULT_SERVER_CONFIG. Read the three locations in packages/MJServer/src/config.ts, packages/MJServer/README.md, and packages/MJCLI/mj.config.cjs, then run pnpm --filter @memberjunction/server run test. Done means one test detects drift and asserts all three values match the server default.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
testing
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.