AI Implementation feature(847): First-Start Create Admin Modal 1.06 (#9)

.kilo/plans/846.md

@@ -1,52 +0,0 @@
-# Implementation Plan: First-Start Create Admin Modal 1.05
-
-## 1. Architectural Reconnaissance
-- **Status:** The requested race-condition fix is **not fully implemented**. `backend/src/modules/setup/setup.service.ts:58-93` performs the admin count check inside a transaction, but two transactions can both observe zero admins before the Argon2 hash and subsequent insert. Existing tests cover serial creation and post-initialization rejection, but not simultaneous different-user submissions or refresh-token cardinality.
-- **Codebase style & conventions:** TypeScript monorepo using NestJS 10, Angular 17 standalone components, async/await, dependency injection, Zod request validation, TypeORM repositories/transactions, and centralized `ApiError`/global exception formatting. Controllers are thin; bootstrap business logic belongs in `SetupService`.
-- **Data Layer:** SQLite through `better-sqlite3`, accessed with TypeORM and explicit migrations (`synchronize: false`). The `user` table has a unique username but no invariant-enforcing uniqueness constraint for the `role` value. Database startup runs migrations and enforces WAL mode. The setup mutation and refresh-token creation already share one TypeORM transaction.
-- **Test Framework & Structure:** Root Jest multi-project configuration (`tests/jest.config.js`) runs backend Node tests from `tests/backend/**/*.spec.ts` and frontend jsdom tests from `tests/frontend/**/*.spec.ts`; tests are kept outside source directories and run with `npm test`. Backend integration tests use Nest `Test.createTestingModule`, the real SQLite TypeORM database, `supertest` via `csrfClient`, and cleanup in `beforeEach`/`afterAll`.
-- **Required Tools & Dependencies:** No new package or system dependency should be required. Continue using the existing Node/npm toolchain, NestJS, TypeORM, `better-sqlite3`, Argon2, Jest, and Supertest. `setup.sh` already installs dependencies and builds both workspaces; do not add infrastructure unless the chosen SQLite locking approach requires an existing package (the preferred plan does not). If a migration is added, it is TypeScript and is included by the existing database migration registration rather than requiring a new CLI.
-
-## 2. Impacted Files
-- **To Modify:**
-  - `backend/src/modules/setup/setup.service.ts` — serialize or otherwise atomically coordinate first-admin bootstrap attempts so only one request can pass the empty-admin gate and complete user/session creation.
-  - `tests/backend/setup-create-admin.spec.ts` — add one focused concurrent-submission regression test asserting one success, one controlled conflict/business-rule failure, exactly one admin, and exactly one refresh-token row.
-  - `docs/api/setup.md` — document the concurrency guarantee and the structured losing-request response if the public contract changes from the currently documented 404/USERNAME_TAKEN cases.
-  - `docs/database/users.md` and/or `docs/guides/bootstrap.md` — update the first-admin invariant description to explicitly cover concurrent requests, if documentation is maintained as part of implementation.
-  - `backend/src/common/errors/error-codes.ts` — only if implementation introduces a dedicated bootstrap-race error code rather than reusing the existing `SYSTEM_INITIALIZED` or `NOT_FOUND` code.
-- **To Create:**
-  - No new application source file is required by the preferred design.
-  - A new migration file is not expected unless the implementer chooses a database-level sentinel/lock-row design; if that design is selected, create a timestamped migration under `backend/src/database/migrations/` and register it in `backend/src/database/database.module.ts`.
-
-## 3. Proposed Changes
-Explain the exact implementation logic step-by-step:
-
-1. **Database / Schema Migration:**
-   - Prefer no schema change: the existing transaction and SQLite WAL database are sufficient if the setup critical section is serialized before the admin count check.
-   - Implement a process-local, promise-based mutex/critical-section guard owned by `SetupService` (or an equivalent injected singleton lock service) around the entire first-admin check-to-commit path. The lock must be acquired before `dataSource.transaction` performs the admin count query and released in a `finally` block on success, validation failure, database failure, or controlled conflict.
-   - Keep the password equality, password-policy validation, rate-limit check, and Argon2 hashing semantics unchanged. The critical requirement is that the second request cannot execute the empty-admin check until the first transaction has either committed the admin and refresh token or rolled back.
-   - Once the first transaction commits, the next request rechecks the admin count and exits without hashing, creating a user, or minting a refresh token. Return a controlled business-rule response. The cleanest contract is the existing `SYSTEM_INITIALIZED` 409 already used by the legacy first-admin endpoint; alternatively retain the setup endpoint's existing `NOT_FOUND` 404 contract if compatibility is more important. Whichever status/code is selected must be consistent and explicitly tested/documented.
-   - Do not rely on the username unique constraint alone: concurrent requests intentionally use different usernames, so that constraint cannot enforce the first-admin invariant. Do not rely on the current unlocked count query or on Argon2's synchronous blocking behavior.
-   - If deployment topology may include multiple Node processes/workers, document that an in-process mutex only protects one process. For a true multi-process guarantee, replace it with a database-backed lock/sentinel approach: add one immutable bootstrap lock row/table via migration, acquire it using SQLite's write serialization/transaction semantics before counting admins, then perform the check, hash, insert, and session mint in the same transaction. Validate the chosen approach against the repository's single-process topology before implementation; the minimum Job reproduction is within one Nest process.
-
-2. **Backend Logic & APIs:**
-   - Update `SetupService.createAdmin` so every path that can mutate bootstrap state is within the same serialized/atomic boundary. Preserve the existing transaction callback ordering: count admins, reject initialized systems, check username, hash password, save enabled admin, record rate limit, and call `AuthService.createSession` using the transaction manager.
-   - Ensure lock release occurs after the transaction promise settles, not immediately after starting it. This prevents the second request from observing an uncommitted or partially completed first request.
-   - Normalize the losing request through `ApiError` with a stable structured `code`, message, and HTTP status. Do not expose raw SQLite `SQLITE_BUSY`/constraint errors. If a database-level locking strategy is used, catch/map expected lock contention to the same business-rule error and ensure failed transactions cannot leave a user or refresh-token row behind.
-   - Leave `SetupController` as a thin delegator. It should continue to derive the IP, call the service, set the HttpOnly refresh cookie only on a successful session, and return the existing 201 response shape for the winner.
-   - Keep frontend behavior unchanged unless the selected loser code is not already handled. `SetupCreateAdminService` already preserves status/code from the API, while `SetupCreateAdminComponent` treats only `USERNAME_TAKEN` as non-retryable and presents a generic retryable error for other codes. If `SYSTEM_INITIALIZED` is chosen, update the component/service contract only if UX should stop retrying after another caller wins; otherwise document that a concurrent loser should be redirected/reloaded rather than repeatedly retrying a now-initialized setup route.
-   - Update the setup API documentation to state: among simultaneous valid first-admin requests, exactly one may return 201; all others receive the chosen structured conflict/business-rule response; only one enabled admin and one refresh-token row are created.
-
-3. **Frontend UI Integration:**
-   - No visual/modal redesign is required. The modal already disables duplicate clicks within one component instance (`submitting` signal), but that does not protect independent browser requests; the invariant must remain server-side.
-   - If backend returns `SYSTEM_INITIALIZED`/409 for the losing request, decide and implement the smallest compatible handling: mark the setup flow unavailable and route to login/bootstrap reload, or retain the existing generic failure presentation. Add a frontend logic test only if a changed error-code branch is introduced; do not add visual/browser tests for this backend race fix.
-
-## 4. Test Strategy
-- **Target Unit Test File:** `tests/backend/setup-create-admin.spec.ts`.
-- Add one minimal regression test using the existing initialized Nest app and real in-memory SQLite database. Clear both users and refresh tokens before the test (the current `beforeEach` only clears users, so the new test must explicitly clean refresh tokens or use a safe test isolation arrangement).
-- Arrange two independent valid requests with different usernames and invoke them concurrently with `Promise.all` through separate `csrfClient` instances or equivalent Supertest agents. Avoid mocking Argon2, TypeORM, or the database: the bug is transaction/concurrency behavior and must be verified at the HTTP/service boundary.
-- Assert that the result statuses contain exactly one `201` and exactly one expected controlled loser status (normally `409`, or `404` if compatibility-preserving behavior is selected). Assert the loser body has the exact structured error `code`; do not assert a generated token value.
-- Assert database postconditions through repositories: exactly one row with `role: 'admin'`, exactly one enabled first-admin user overall, and exactly one row in `refresh_token`. Verify the winning username is one of the two submitted names and the losing username is absent. This directly guards against both duplicate admins and duplicate sessions.
-- Retain the existing focused tests for winner success, serial post-initialization rejection, weak passwords, mismatch, and invalid usernames. Update only assertions/comments that become stale if the selected concurrency error code changes.
-- Follow AAA structure, clear mocks if any are introduced, close the Nest app, and ensure no shared persistent data is written. Use the existing `DATABASE_PATH=':memory:'`; `/data` is unnecessary because the test does not create successor-job assets or persistent fixtures.
-- Run the repository's single root command `npm test` during implementation, plus the project's available build/type validation commands discovered from package scripts. There is no lint or typecheck script in the inspected root/backend package manifests; do not invent one. `setup.sh` needs no update for the planned implementation.

.kilo/plans/847.md

@@ -0,0 +1,107 @@
+# 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.

docs/api/setup.md

@@ -3,7 +3,7 @@ type: api
 title: Setup Endpoint
 description: Dedicated POST /api/v1/setup/create-admin endpoint used only while no administrator exists.
 tags: [api, setup, bootstrap, first-admin]
-timestamp: 2026-07-21T17:18:08Z
+timestamp: 2026-07-21T17:37:18Z
 ---
 
 # Endpoint

docs/database/users.md

@@ -3,7 +3,7 @@ type: database
 title: User Table
 description: The `user` table — accounts, roles, and statuses.
 tags: [database, user, auth]
-timestamp: 2026-07-21T14:43:00Z
+timestamp: 2026-07-21T17:37:18Z
 ---
 
 # Schema
@@ -37,14 +37,30 @@ timestamp: 2026-07-21T14:43:00Z
 When the `user` table is empty of admins, the dedicated
 `POST /api/v1/setup/create-admin` endpoint (see
 [Setup Endpoint](/api/setup.md)) creates the very first admin and
-auto-issues a session. Once any admin row exists the endpoint becomes
-inert and returns `404 NOT_FOUND`, so the route is effectively hidden
-in production.
+auto-issues a session. Once any admin row exists the endpoint rejects
+further attempts with `409 SYSTEM_INITIALIZED`, so the route is
+effectively hidden in production.
 
 The legacy `POST /api/v1/auth/register-first-admin` route is preserved
 for compatibility but the canonical first-admin flow now lives in the
 `SetupModule`.
 
+# Concurrent bootstrap safety
+
+Two near-simultaneous `POST /api/v1/setup/create-admin` requests cannot
+both succeed. `SetupService.createAdmin` serializes callers on an
+in-process promise chain (`#bootstrapChain` + private
+`runBootstrap()` in `backend/src/modules/setup/setup.service.ts:111-130`)
+so that the first request commits inside its `DataSource.transaction`
+before the second request's transaction executes. The loser's
+transaction observes `adminCount > 0` and throws
+`ApiError(SYSTEM_INITIALIZED, 409)`; the winner is the only request that
+creates a `UserEntity(role='admin')` row and mints a refresh token.
+The lock is per-process; multi-replica deployments rely on the unique
+index and transaction guard, not on this chain. The regression test is
+`tests/backend/setup-create-admin.spec.ts` ("only one of two concurrent
+first-admin requests succeeds").
+
 # See also
 
 - [Auth and Settings Tables](/database/auth-settings.md)

docs/index.md

@@ -11,7 +11,7 @@ scoreboard, an event window with a public countdown, theming, and admin
 controls.
 
 The docs below are organized by purpose so agents can pull just the slice
-they need. Last regenerated 2026-07-21T17:18:08Z.
+they need. Last regenerated 2026-07-21T17:37:18Z.
 
 # Architecture