108 lines
5.1 KiB
Markdown
108 lines
5.1 KiB
Markdown
# Implementation Plan: First-Start Create Admin Modal 1.06 — Bootstrap Race Fix
|
|
|
|
## 0. Status
|
|
|
|
**[ALREADY_IMPLEMENTED]**
|
|
|
|
The race described in Job 847 is already fixed in the current codebase. Two
|
|
near-simultaneous `POST /api/v1/setup/create-admin` requests can no longer
|
|
both succeed: exactly one returns HTTP 201 and the other returns HTTP 409
|
|
`SYSTEM_INITIALIZED`. No new code or test work is required.
|
|
|
|
## 1. Architectural Reconnaissance
|
|
|
|
- **Codebase style & conventions:** TypeScript, NestJS 10+ (modular, DI,
|
|
controllers + services), class-based services, TypeORM with the
|
|
`DataSource.transaction` API, `zod` DTOs guarded by
|
|
`ZodValidationPipe`, a project-wide `ApiError` / `ERROR_CODES` enum,
|
|
and Argon2id password hashing.
|
|
- **Data Layer:** SQLite (in-memory for tests, file-based for runtime),
|
|
TypeORM entities; the bootstrap flow wraps all admin creation inside
|
|
a single `DataSource.transaction` so the `adminCount > 0` guard and
|
|
the user insert commit atomically.
|
|
- **Test Framework & Structure:** Jest. Tests live exclusively under
|
|
`/repo/tests/` (separated into `tests/backend/` and `tests/frontend/`),
|
|
never alongside source. Backed by a `csrfClient` helper in
|
|
`tests/backend/csrf-client.ts` and an `initDb(app)` helper that uses
|
|
SQL migrations to provision the in-memory database. Tests are
|
|
runnable from the repo root via the standard `npm test` pipeline.
|
|
- **Required Tools & Dependencies:** None beyond the existing stack
|
|
(NestJS, TypeORM, better-sqlite3, argon2, zod, Jest, Supertest,
|
|
uuid). No new package, system tool, or `setup.sh` change is needed.
|
|
|
|
## 2. Impacted Files
|
|
|
|
The implementation already exists in these files; no further edits are
|
|
required:
|
|
|
|
- **To Modify:** None.
|
|
- **To Create:** None.
|
|
- **Existing files that already contain the fix (read-only references):**
|
|
- `backend/src/modules/setup/setup.service.ts:23,39-97,111-130` —
|
|
`SetupService.createAdmin` runs every bootstrap attempt through
|
|
`runBootstrap()`, which serializes callers on a per-process promise
|
|
chain (`#bootstrapChain`) so two concurrent transactions cannot
|
|
both observe an empty `user` table.
|
|
- `backend/src/modules/setup/setup.service.ts:60-95` — Each
|
|
serialized call executes inside `dataSource.transaction(...)`,
|
|
performs the `adminCount > 0` check first, throws
|
|
`ApiError(SYSTEM_INITIALIZED, 409)` on the loser, then performs the
|
|
`username` uniqueness check, Argon2id hash, and `UserEntity`
|
|
insert on the winner.
|
|
- `backend/src/modules/setup/setup.module.ts` — Wires
|
|
`RegistrationRateLimitService`, `JwtService`, `ConfigService`,
|
|
`DataSource`, and the user/refresh-token repositories into
|
|
`SetupService`.
|
|
- `backend/src/common/errors/api-error.ts`,
|
|
`backend/src/common/errors/error-codes.ts` — Provide `SYSTEM_INITIALIZED`
|
|
and the 409 mapping consumed by `GlobalExceptionFilter`.
|
|
- `tests/backend/setup-create-admin.spec.ts:121-149` — Existing
|
|
regression test `"only one of two concurrent first-admin requests
|
|
succeeds; the other receives a controlled conflict"` already
|
|
reproduces the race described in the Job and asserts
|
|
`[201, 409]`, the loser's `SYSTEM_INITIALIZED` code, exactly one
|
|
admin row, and exactly one `refresh_token` row.
|
|
|
|
## 3. Proposed Changes
|
|
|
|
No changes. The Job's expected behaviour is already enforced by two
|
|
cooperating layers:
|
|
|
|
1. **Database / Schema Migration:** None. The existing `user.role`
|
|
index plus the in-transaction `adminCount` check are sufficient.
|
|
2. **Backend Logic & APIs:** `SetupService.createAdmin` already wraps
|
|
the whole bootstrap pipeline in `runBootstrap()` which serializes
|
|
callers on a single process-local promise chain, and each task
|
|
itself runs inside a `DataSource.transaction` whose first action is
|
|
the `adminCount > 0` guard. The controller path
|
|
`POST /api/v1/setup/create-admin` already maps the
|
|
`SYSTEM_INITIALIZED` `ApiError` to HTTP 409 via
|
|
`GlobalExceptionFilter`.
|
|
3. **Frontend UI Integration:** No change needed. The frontend service
|
|
`frontend/src/app/features/setup/setup-create-admin.service.ts`
|
|
already switches on `CreateAdminResult` and surfaces 409 as the
|
|
"system already initialized" inline error with a Retry affordance.
|
|
|
|
## 4. Test Strategy
|
|
|
|
The required regression test already exists and is the test that proves
|
|
the fix. No additional tests are planned because:
|
|
|
|
- The failing scenario in the Job (two concurrent `adminAlpha` /
|
|
`adminBeta` requests both returning 201 with admin rows + two
|
|
`refresh_token` rows) is exactly the scenario covered by
|
|
`tests/backend/setup-create-admin.spec.ts:121-149`.
|
|
- Adding a second concurrent-race test would duplicate coverage without
|
|
exercising new behavior, violating the "minimal, highly-focused tests"
|
|
constraint.
|
|
|
|
If a future run of `npm test` ever fails this spec, the implementer
|
|
should investigate `SetupService.runBootstrap` and the transactional
|
|
`adminCount` guard (not add tests).
|
|
|
|
### Verification commands (for the implementer only — not for execution here)
|
|
|
|
- `npm test -- tests/backend/setup-create-admin.spec.ts` — confirms
|
|
201/409 outcome, single admin row, single `refresh_token` row.
|
|
- `npm test` — full Jest run from the repo root.
|