.kilo/plans/897.md

@@ -1,85 +0,0 @@
-# Implementation Plan: Admin Edit Category — System Row Pre-fill Bug (Job 897)
-
-## 1. Architectural Reconnaissance
-
-- **Codebase style & conventions:**
-  - **Stack:** Node.js monorepo (npm workspaces) — `backend/` (NestJS 10, TypeScript, better-sqlite3) + `frontend/` (Angular 20 standalone components, signals, signal inputs, OnPush change detection, reactive forms via `FormBuilder.nonNullable`, `ReactiveFormsModule`).
-  - **Frontend patterns:** Standalone components, signal inputs (`input()`, `output()`), `effect()` for input-driven state synchronization, `ChangeDetectionStrategy.OnPush`, lazy-loaded feature routes, typed `FormGroup<T>` with `nonNullable` controls.
-  - **Backend patterns:** NestJS modules/controllers/services, zod DTOs with class-validator, sqlite migrations, `AdminGuard` + `@Roles('admin')`.
-  - **Conventions:** Test IDs are stable hooks (`data-testid="cf-name"`, `cf-abbr"`, `cf-desc"`, `cat-edit-{ABBR}`). Components are pure dumb/presentational where possible — `CategoryFormModalComponent` only owns its form state and emits `submit`/`cancel`; the parent (`AdminCategoriesComponent`) handles API calls.
-- **Data Layer:** SQLite (better-sqlite3) accessed via Knex/raw queries. The `category` table already has `system_key`, `created_at`, `updated_at`, `icon_path` and `is_system` is derived from `system_key IS NOT NULL`. No schema migration is needed for this bug — the data is correct, the UI client fails to display it.
-- **Test Framework & Structure:**
-  - Jest via `ts-jest`, two projects in `tests/jest.config.js` (`backend` / `frontend`).
-  - **Frontend tests live in `tests/frontend/`** and are pure-function tests (no `TestBed` anywhere in the suite). The existing `tests/frontend/admin-categories-form-modal.spec.ts` already exercises `syncCategoryForm` in isolation. The pattern is to write small, pure helpers that can be unit-tested without DOM.
-  - Single command from root: `npm test` (runs both projects). `npm run test:frontend` for frontend only.
-- **Required Tools & Dependencies:** None new. All dependencies already present in `package.json` workspaces. Angular forms (`@angular/forms`), `@angular/core` (signals/`effect`), and `ts-jest` are already installed.
-
-## 2. Impacted Files
-
-- **To Modify:**
-  - `frontend/src/app/features/admin/categories/category-form-modal.component.ts` — bind the reactive `FormGroup` to the `<form>` element so the `formControlName` directives can resolve it; this is the root cause of the `Cannot read properties of null (reading 'addControl')` error and the empty input fields.
-  - `tests/frontend/admin-categories-form-modal.spec.ts` — add focused regression coverage for the modal's prefill behavior so the bug cannot silently regress again.
-- **To Create:**
-  - None. (No backend, no DTO, no migration, no new service, no new component.)
-
-## 3. Proposed Changes
-
-### Root cause analysis
-
-In `category-form-modal.component.ts` line 67 the template is:
-
-```html
-<form class="modal" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
-```
-
-There is **no `[formGroup]="form"` binding** on the `<form>` element. The three inputs/textarea inside use `formControlName="name"`, `formControlName="abbreviation"`, and `formControlName="description"`. Without the `formGroup` directive on the parent `<form>`, Angular's `FormControlName` directive (which is what `formControlName` compiles to) cannot find a parent `FormGroup`/`FormGroupDirective`, so:
-
-1. It throws `TypeError: Cannot read properties of null (reading 'addControl')` from `FormControlName._setUpControl` / `ngOnChanges` — exactly the error reported in the browser console.
-2. The DOM `<input>` elements have no view-to-model binding, so they always render empty regardless of what `syncCategoryForm` writes into the `form` model.
-3. The "icon preview" element bound to `iconPreview()` appears, but is never updated because the same `effect()` that drives `iconPreview` is also re-asserting the (now non-bit) `abbreviationReadonly` signal — in effect the form was technically being filled, but the inputs are disconnected from the model.
-
-This is *also* why switching from `cat-edit-CRY` to `cat-edit-HW` does not change the icon preview: the controls' DOM values were never under the form's control, but the icon preview `<img>` re-binds correctly — except the bug report says it stays on `CRY.png`. That same symptom is consistent with the modal being re-mounted (because the inner `@if (open())` block recreates the `<form>`) **before** the next `effect` flush writes the new `iconPreview` value, OR the user is reading the `<img>` cache and the network 404 icon has been cached. The primary fix (binding `[formGroup]`) repairs the form-control wiring; the icon preview re-render is then guaranteed by the same `effect` that already runs `this.iconPreview.set(result.iconPreview)` correctly.
-
-### Step-by-step implementation
-
-1. **Bind the form group directive** (the only production change).
-   - In `frontend/src/app/features/admin/categories/category-form-modal.component.ts` template, change the `<form>` opening tag from:
-     ```html
-     <form class="modal" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
-     ```
-     to:
-     ```html
-     <form class="modal" [formGroup]="form" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
-     ```
-   - This is a one-line, surgical change. It does not alter any signal, effect, model, or API contract. It re-establishes the missing reactive-forms directive parent that the three `formControlName` attributes already assume.
-
-2. **Verify the existing `syncCategoryForm` helper is still the single source of truth.**
-   - The constructor `effect()` at lines 128–137 already calls `syncCategoryForm(this.form, this.mode(), this.category())` synchronously and also sets `abbreviationReadonly`/`iconPreview` signals and `markForCheck()`. No edits to `syncCategoryForm` itself are needed.
-   - After step 1, when the parent sets `[open]=true [mode]="edit" [category]="category"`, the effect (a) writes values into the form model, and (b) Angular's change detection now correctly propagates those values into the `<input>`s because the `[formGroup]` directive is in place.
-
-3. **No backend changes.**
-   - `GET /api/v1/admin/categories` already returns `{ id, name, abbreviation, description, iconPath, isSystem, ... }` correctly. The `CategoriesService.list` returns rows sorted by `LOWER(abbreviation)` ascending, so the six system rows are already returned in the order the bug report describes.
-   - `PUT /api/v1/admin/categories/:id` already accepts `{ name?, description?, iconPath? }` for system rows (the abbreviation is locked client-side and re-validated server-side via `SYSTEM_PROTECTED`). No DTO change required.
-
-4. **No schema / migration.**
-   - All required columns already exist.
-
-5. **No new dependencies and no `setup.sh` change.** The fix is pure frontend.
-
-### Why this is the minimal correct fix
-
-- The bug report itself identifies the symptom: `TypeError: Cannot read properties of null (reading 'addControl') at i._setUpControl / i.ngOnChanges`. That error is generated exclusively by `FormControlName`/`FormGroupDirective` parent-resolution failure. The only `<form>` in the modal template is missing `[formGroup]`. Adding the binding resolves the error and the empty inputs in one shot.
-- The "icon preview stuck on CRY.png" observation is a secondary effect of the same root cause: without a working form, the modal's lifecycle is unstable; the fix re-stabilizes it and the same `effect` that sets `iconPreview` already writes the new value on every `category()` change.
-- All other parts of the prefill pipeline (`nonNullable` controls, `setValue(..., { emitEvent: false })`, pristine/untouched markers, `markForCheck()` on OnPush, signal-driven `abbreviationReadonly` for system rows) are already correct.
-
-## 4. Test Strategy
-
-- **Target Unit Test File:** `tests/frontend/admin-categories-form-modal.spec.ts` (existing, pure-helper tests).
-- **Mocking Strategy:** No mocks required. The existing test file already exercises `syncCategoryForm` directly with a hand-built `FormGroup` and `makeRow()` helper. We add focused, fast assertions that lock in the prefill contract that the modal template now relies on. The fix is a single template binding, so the regression test is best expressed at the helper boundary plus a small assertion that the component's `form` field is exposed with the expected control names (so any future change that re-introduces the bug — e.g. someone removing `formControlName` or re-binding to a different form — is caught by the test that asserts the visible class contract).
-- **Tests to add (kept minimal, single-pass friendly):**
-  1. `it('exposes a form group with name, abbreviation, description controls')` — instantiate the component via `new CategoryFormModalComponent()` after manually constructing the `FormBuilder` (mirror the existing `buildForm()` pattern), assert `form.controls.name`, `form.controls.abbreviation`, `form.controls.description` exist and are non-null. This is the contract the template now relies on (before the fix, the template silently failed to bind).
-  2. `it('keeps the prefill behaviour for a system row written by the constructor effect')` — drive the public effect manually with `(mode='edit', category=makeRow())` and assert all three controls hold the seeded values *and* `abbreviationReadonly()` signal is `true`. This catches the exact regression: the user reports empty inputs after clicking `cat-edit-CRY`, with `abbreviationReadonly` true.
-  3. `it('updates iconPreview when switching between two system rows')` — call `syncCategoryForm` (via the same effect pathway) once with `CRY` (`/uploads/icons/CRY.png`) then again with `HW` (`/uploads/icons/HW.png`); assert `iconPreview()` (a public signal) reflects the new value. This locks the "icon preview stuck on CRY.png" symptom.
-- **Run command:** `npm run test:frontend` from the repo root (or `npm test` to run both projects). The new specs run in <100 ms because they avoid `TestBed` and DOM rendering, consistent with the rest of the frontend suite.
-- **No UI / no visual verification required.** Per the project rules, frontend tests focus on logic only.
-- **No `/data` usage is required** for this job — there is no seeded data, no DB seed, and no persistence to test.

.kilo/plans/898.md

@@ -0,0 +1,35 @@
+# Implementation Plan: Admin Area General Settings and Categories 1.16
+
+## 1. Architectural Reconnaissance
+- **Codebase style & conventions:** TypeScript monorepo with a NestJS 10 REST backend and Angular 17 standalone components. Backend controllers currently delegate category persistence to TypeORM-backed services and validate JSON DTOs with Zod pipes; uploads use Nest `FileInterceptor`, in-memory Multer buffers, and Sharp. Frontend admin pages use standalone OnPush components, signals, typed reactive forms, and an `AdminService` wrapper around typed `HttpClient` calls converted to promises with `firstValueFrom`.
+- **Data Layer:** SQLite through TypeORM. Category abbreviations are stored uniquely, are normalized to uppercase by `AdminCategoriesService`, and list queries order by `LOWER(abbreviation)`. No schema migration is needed because `UpdateCategorySchema`, the entity, and service update path already support abbreviation changes for non-system categories.
+- **Test Framework & Structure:** Jest 29 with `ts-jest`, split into root-level dedicated `tests/backend` and `tests/frontend` projects and runnable together with root `npm test`. Backend endpoint tests use Nest's testing module, Supertest, an in-memory SQLite database, and an isolated upload directory; frontend tests favor exported pure component helpers rather than browser or visual checks.
+- **Required Tools & Dependencies:** No new system tools, global CLIs, or package dependencies are required. Sharp is already installed in both the backend and root test dependencies and can validate/decode and normalize uploaded images. Existing `setup.sh` already creates persistent `/data/hipctf/uploads`, installs dependencies, rebuilds the native SQLite binding, and builds both workspaces, so no setup change is required.
+
+## 2. Impacted Files
+- **To Modify:**
+  - `frontend/src/app/features/admin/categories/categories.component.ts` — include the submitted abbreviation in user-category update requests, preserve the modal on failure, and route save/upload errors to form-modal state rather than delete-modal state.
+  - `frontend/src/app/features/admin/categories/category-form-modal.component.ts` — accept and render a clear save/upload error and accurately expose submission state while the parent performs the async operation.
+  - `backend/src/modules/uploads/uploads.controller.ts` — reject undecodable/non-image category-icon buffers before any filesystem write instead of falling back to raw bytes.
+  - `tests/frontend/admin-categories-form-modal.spec.ts` — add minimal logic assertions for user-category abbreviation editability/submission and form error-state contract as appropriate to the extracted helpers.
+  - `tests/backend/uploads.spec.ts` — replace the legacy text-file success expectation with a real valid image success case and add focused corrupt/non-image rejection checks that assert no overwrite or new file occurs.
+- **To Create:** None.
+
+## 3. Proposed Changes
+1. **Database / Schema Migration:** No database or migration changes. Retain the existing `UpdateCategorySchema.abbreviation` field and `AdminCategoriesService.update()` behavior, which uppercases non-system abbreviations, rejects duplicates, and protects system abbreviations. After a successful client save, the existing reload and defensive sort will move the renamed row into alphabetical order.
+2. **Backend Logic & APIs:**
+   - In the existing `POST /api/v1/uploads/category-icon` flow, treat Sharp decode/resize failure as a `BadRequestException` with a clear image-validation message; do not use multipart MIME type, filename extension, or browser `accept` metadata as proof of validity.
+   - Decode and normalize the in-memory buffer completely to a 128x128 PNG before creating directories or calling `writeFileSync`. Only after Sharp succeeds should the deterministic `{categoryId}.png` path be overwritten. This preserves any previously valid icon when a replacement is corrupt and prevents partial/new files for invalid uploads.
+   - Return the existing successful response contract (`publicUrl`, `width: 128`, `height: 128`, `mimeType: image/png`, plus current metadata fields) so `AdminService.uploadCategoryIcon()` remains compatible. Keep the existing size and missing-file checks unchanged.
+   - Keep category persistence separate from upload persistence: on edit, the frontend must not dispatch the category update after upload rejection, so `updatedAt` and other fields remain unchanged. On create, retain the current create-then-upload architecture; if upload fails, no image or icon-path update is created, although the category row already created by the existing API remains. Do not introduce a database migration or new multipart category-creation endpoint outside this job's reported edit/upload defects.
+3. **Frontend UI Integration:**
+   - Extend the edit branch of `AdminCategoriesComponent.onFormSubmit()` to pass `abbreviation: payload.abbreviation` alongside name, description, and icon path. The modal already uppercases the emitted value, while the server remains the canonical normalizer and protects system rows.
+   - Preserve the current `syncCategoryForm()` contract: user rows remain editable (`readonly=false`) and system rows remain locked (`readonly=true`). Do not mark user-category abbreviation readonly, because the required behavior is to persist it rather than silently discard it.
+   - Add dedicated category-form error state in the container, clear it when opening/closing or beginning a submission, and bind it into the form modal. On upload or update failure, keep the edit modal open, show the server's clear error, and leave the existing list/icon unchanged; do not reuse `deleteError`, which is only rendered by the delete modal.
+   - Coordinate a parent-managed saving flag with the modal so repeated submissions are disabled during upload/update and reset in `finally`. The modal should render the bound error using a stable test id and continue to emit a typed `CategoryFormSubmit`; no HTTP calls move into the presentational component.
+
+## 4. Test Strategy
+- **Target Unit Test File:**
+  - `tests/backend/uploads.spec.ts`: use Sharp to generate one tiny valid image and verify category-icon normalization succeeds; upload a plain-text payload and a corrupt PNG using a deterministic `categoryId`, expect HTTP 400 with the validation message, and assert the target icon is absent or that pre-existing valid bytes are unchanged.
+  - `tests/frontend/admin-categories-form-modal.spec.ts`: retain pure, CLI-only tests and add only the core contract checks needed to prove a non-system edit stays writable and produces/forwards an uppercase abbreviation, plus that a supplied save error is exposed without closing/resetting the form. If submission mapping is not currently isolatable, extract a small typed pure helper from the modal/container rather than building a large TestBed environment.
+- **Mocking Strategy:** Backend endpoint tests use the real Nest route, Sharp, and filesystem boundary in the suite's isolated temporary upload directory, with generated buffers and explicit cleanup; they do not mock image decoding or require `/data`. Frontend tests mock no browser UI and exercise exported pure helpers/typed payload mapping directly. Run all tests from `/repo` with `npm test`; then run the existing workspace builds (`npm --workspace frontend run build` and `npm --workspace backend run build`) as type/build verification. No visual confirmation or external services are required.

backend/src/modules/uploads/uploads.controller.ts

@@ -65,6 +65,9 @@ export class UploadsController {
   private static readonly LOGO_ERROR_MESSAGE =
     'Logo must be a valid PNG, JPEG, GIF, or WebP image.';
 
+  private static readonly CATEGORY_ICON_ERROR_MESSAGE =
+    'Category icon must be a valid PNG, JPEG, GIF, or WebP image.';
+
   private async handleCategoryIcon(req: any): Promise<{ id: string; publicUrl: string; width: number; height: number; mimeType: string; storedPath: string; size: number; originalFilename: string }> {
     const file = req.file as any;
     if (!file) throw new BadRequestException('No file uploaded under field "file"');
@@ -82,25 +85,19 @@ export class UploadsController {
     const fileName = hasId ? `${categoryId}.png` : safeFilename(file.originalname || 'file.png');
     const finalPath = path.join(finalDir, fileName);
 
-    let width = 0;
-    let height = 0;
-    let mimeType = file.mimetype || 'application/octet-stream';
-    let stored: Buffer = file.buffer;
+    let stored: Buffer;
     try {
       stored = await sharp(file.buffer)
         .resize(128, 128, { fit: 'cover', position: 'centre' })
         .png()
         .toBuffer();
-      width = 128;
-      height = 128;
-      mimeType = 'image/png';
     } catch {
-      // Sharp could not parse this buffer as a supported image format
-      // (this is the case for the legacy integration-test fixture which
-      // uploads text bytes). Fall back to storing the raw payload so the
-      // upload endpoint remains available to legacy callers.
+      throw new BadRequestException(UploadsController.CATEGORY_ICON_ERROR_MESSAGE);
     }
     fs.writeFileSync(finalPath, stored);
+    const width = 128;
+    const height = 128;
+    const mimeType = 'image/png';
     return {
       id: fileName,
       originalFilename: file.originalname || 'file.png',

docs/api/uploads.md

@@ -3,7 +3,7 @@ type: api
 title: Uploads Endpoints
 description: Admin-only multipart upload endpoints for site logos, category icons, and challenge files.
 tags: [api, uploads, multipart, admin]
-timestamp: 2026-07-22T13:05:30Z
+timestamp: 2026-07-22T19:18:00Z
 ---
 
 # Endpoints
@@ -37,6 +37,19 @@ valid admin session and the standard CSRF cookie/header pair.
 * An accepted logo keeps its sanitized filename, is written directly under
   `UPLOAD_DIR`, and returns `{ publicUrl, originalFilename }`, with `publicUrl`
   in the form `/uploads/{safeFilename}`.
+* Category icons are decoded with Sharp and normalized to a 128-by-128 PNG.
+  When `categoryId` is supplied, the file is written to
+  `UPLOAD_DIR/icons/{categoryId}.png` (overwriting any previous icon for that
+  category) and the response carries `{ id: "{categoryId}.png", publicUrl,
+  width: 128, height: 128, mimeType: "image/png", storedPath, size,
+  originalFilename }`.
+* Category-icon validation is strict: a missing `file`, an oversized upload,
+  a corrupt/non-image payload, or a buffer that Sharp cannot decode returns
+  `400 Bad Request`. Invalid category-icon payloads use the message
+  `Category icon must be a valid PNG, JPEG, GIF, or WebP image.` and **are
+  not persisted** — the existing icon file (if any) on disk is left untouched.
+  The multipart MIME type, the filename extension, and any browser `accept`
+  metadata are not trusted as proof of validity.
 * A missing `file` part, an oversized upload, a corrupt image, or a decoded logo
   in another format returns `400 Bad Request`. Invalid logo payloads use the
   message `Logo must be a valid PNG, JPEG, GIF, or WebP image.` and are not

docs/architecture/key-files.md

@@ -3,7 +3,7 @@ type: architecture
 title: Key Files Index
 description: One-line responsibility for important source and contract-test files, including strict event-window validation and the public bootstrap SSE listener.
 tags: [architecture, key-files, event-window, validation, default-challenge-ip, sse, bootstrap, migrations]
-timestamp: 2026-07-22T18:37:00Z
+timestamp: 2026-07-22T19:18:00Z
 ---
 
 # Backend
@@ -25,7 +25,7 @@ timestamp: 2026-07-22T18:37:00Z
 | `backend/src/modules/admin/general.service.ts` | Reads/writes global settings, filters available theme files, and publishes the `general` SSE notification after updates. |
 | `backend/src/modules/auth/auth.controller.ts` | Registers authentication and account endpoints. |
 | `backend/src/modules/auth/auth.service.ts` | Handles sessions, authentication, registration, and password changes. |
-| `backend/src/modules/uploads/uploads.controller.ts` | Registers admin-only multipart uploads, including Sharp-backed site-logo format validation. |
+| `backend/src/modules/uploads/uploads.controller.ts` | Registers admin-only multipart uploads, including Sharp-backed site-logo format validation and strict category-icon decode (rejects undecodable / non-image buffers with HTTP 400 before any filesystem write so existing icons are preserved). |
 | `backend/src/modules/admin/dto/general.dto.ts` | Zod contract for `PUT /api/v1/admin/general/settings`; both event timestamps are required ISO-8601 values and end must be strictly after start. |
 | `tests/backend/admin-general-event-window.spec.ts` | Focused contract tests for valid, empty, malformed, equal, and reversed event-window timestamps. |
 | `tests/backend/admin-general-service.spec.ts` | General-settings schema and service tests, including per-field empty datetime failures and settings-event emission. |
@@ -52,7 +52,7 @@ timestamp: 2026-07-22T18:37:00Z
 | `frontend/src/app/features/shell/tabs/quick-tabs.component.ts` | Main shell navigation tabs. |
 | `frontend/src/app/features/shell/change-password/change-password-modal.component.ts` | Change-password form modal. |
 | `frontend/src/app/features/admin/general.component.ts` | `AdminGeneralComponent` reactive form for `/admin/general` — per-field inline error rendering (page-title + event-start + event-end), logo upload wiring, welcome Markdown preview, event-state derivation, and SSE `general` event handling. |
-| `frontend/src/app/features/admin/categories/category-form-modal.component.ts` | Standalone OnPush modal for create + edit; owns the `CategoryFormGroup`, exposes the pure `syncCategoryForm` helper, and reacts to `open` / `mode` / `category` signal inputs via a `markForCheck` effect so edit prefill reaches the DOM. |
+| `frontend/src/app/features/admin/categories/category-form-modal.component.ts` | Standalone OnPush modal for create + edit; owns the `CategoryFormGroup`, exposes the pure `syncCategoryForm` helper, binds `[formGroup]` on its template `<form>`, accepts `errorMessage` + `saving` parent inputs (renders `cf-error` and disables OK while saving), and reacts to `open` / `mode` / `category` signal inputs via a `markForCheck` effect so edit prefill reaches the DOM. |
 | `frontend/src/app/features/admin/general.pure.ts` | Pure General Settings helpers, including required datetime validation, field messages, UTC conversion, end-after-start validation, and the default-challenge-address IPv4/hostname validator/normalizer/message trio. |
 | `tests/frontend/admin-general-pure.spec.ts` | Pure client-contract tests for required event timestamps, datetime messaging, UTC conversion, event-window ordering, and default-challenge-address validation, error mapping, and normalization. |
 | `tests/frontend/admin-categories-form-modal.spec.ts` | Tests the pure `syncCategoryForm` helper that drives the edit/create prefill in `CategoryFormModalComponent`: system-row abbreviation lock, user-row unlock, re-population on second invocation, clearing on create, and iconPreview passthrough. |

docs/guides/admin-categories.md

@@ -3,7 +3,7 @@ type: guide
 title: Admin — Categories
 description: How an admin lists, creates, edits, and deletes challenge categories from the /admin/categories page, including system-row protection and challenge-attached protection.
 tags: [guide, admin, categories, tester]
-timestamp: 2026-07-22T18:37:00Z
+timestamp: 2026-07-22T19:18:00Z
 ---
 
 # When this view is available
@@ -45,6 +45,7 @@ side-nav (categories are also embedded inside the General settings page).
 | Form modal             | `[data-testid="cat-form-modal"]`           | Create/edit dialog (`cat-form-backdrop` is the click-outside dismiss layer). |
 | Delete modal           | `[data-testid="cat-delete-modal"]`         | Confirmation dialog (`cat-delete-backdrop` is the dismiss layer). |
 | Delete error           | `[data-testid="cat-delete-error"]`         | Inline error message after a failed delete. |
+| Form error             | `[data-testid="cf-error"]`                 | Inline error rendered inside the form modal after a failed create/update/upload. |
 
 # Form modal fields
 
@@ -54,7 +55,7 @@ side-nav (categories are also embedded inside the General settings page).
 | Abbreviation (uppercase)      | `cf-abbr`      | Required, 2–6 chars; server upper-cases on save. `readonly` when editing a system row. |
 | Description                   | `cf-desc`      | Optional, max 2000 chars. |
 | Icon (file picker)            | `cf-icon`      | Optional image; uploaded to `POST /api/v1/uploads/category-icon` and resized/normalized server-side. |
-| Cancel / OK                   | `cf-cancel`, `cf-ok` | OK disabled while form invalid or already submitting. |
+| Cancel / OK                   | `cf-cancel`, `cf-ok` | OK disabled while form invalid, already submitting, or while the parent is performing the async save (bound `saving` input). |
 
 # Expected behavior
 
@@ -84,10 +85,23 @@ side-nav (categories are also embedded inside the General settings page).
 
 1. Click `cat-edit-{ABBR}` → modal opens in edit mode, pre-filled with
    the row's values.
-2. For system rows, the abbreviation field is `readonly`.
-3. On save the component issues `updateCategory(id, {...})`. If an icon
-   file is selected, the new icon is uploaded first and the returned
-   `publicUrl` replaces `iconPath` in the same update.
+2. For system rows, the abbreviation field is `readonly`. For user
+   (non-system) rows the abbreviation is editable and is uppercased
+   server-side on save (the existing category row is re-sorted
+   alphabetically by the defensive client sort after a successful
+   update).
+3. On save the component issues `updateCategory(id, {...})` with
+   `name`, `abbreviation`, `description`, and (if an icon file was
+   chosen) `iconPath`. When an icon file is selected, the new icon is
+   uploaded first via `POST /api/v1/uploads/category-icon` and the
+   returned `publicUrl` replaces `iconPath` in the same update.
+4. On either an upload rejection (HTTP 400) or an update rejection
+   (HTTP 400/404/409), the modal stays open, the OK button is disabled
+   while the request is in flight via the bound `saving` flag, and the
+   server message is rendered inside the modal at
+   `data-testid="cf-error"`. The list is not refreshed and any
+   previously-valid icon on disk is preserved (the upload endpoint
+   does not overwrite when Sharp decode fails).
 
 ### Edit prefill mechanics
 
@@ -112,6 +126,29 @@ input. `syncCategoryForm` is exported from the modal file so it can be
 exercised in isolation by
 `tests/frontend/admin-categories-form-modal.spec.ts`.
 
+### Save error and loading state
+
+`CategoryFormModalComponent` is purely presentational and does not own
+HTTP state. It exposes two additional signal inputs:
+
+* `errorMessage: string | null` — when truthy, an inline error paragraph
+  is rendered at `data-testid="cf-error"` showing the supplied message.
+* `saving: boolean` — when `true`, the OK button is disabled in addition
+  to the existing `form.invalid` and `submitting` guards.
+
+The parent `AdminCategoriesComponent` owns both signals:
+
+* `formError` — set from the server's `error.message` (or a generic
+  fallback) when create/update/upload fails, and cleared every time the
+  modal is opened or a submission begins.
+* `saving` — set to `true` at the start of `onFormSubmit` and reset to
+  `false` in `finally`, so the modal can never get stuck in a
+  "submitting" state if the request rejects.
+
+This separation matters because `deleteError` (rendered by the delete
+modal at `data-testid="cat-delete-error"`) is a different state machine
+and must not be reused for create/update failures.
+
 ## Delete behavior
 
 * **System rows:** the delete modal renders

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-22T18:37:00Z.
+they need. Last regenerated 2026-07-22T19:18:00Z.
 
 # Architecture
 

frontend/src/app/features/admin/categories/categories.component.ts

@@ -53,6 +53,8 @@ export type ModalMode = 'create' | 'edit';
         [open]="modalMode() !== null"
         [mode]="modalMode() === 'edit' ? 'edit' : 'create'"
         [category]="editing()"
+        [errorMessage]="formError()"
+        [saving]="saving()"
         (cancel)="closeModal()"
         (submit)="onFormSubmit($event)"
       />
@@ -79,6 +81,8 @@ export class AdminCategoriesComponent implements OnInit {
   readonly editing = signal<AdminCategory | null>(null);
   readonly deleting = signal<AdminCategory | null>(null);
   readonly deleteError = signal<string | null>(null);
+  readonly formError = signal<string | null>(null);
+  readonly saving = signal(false);
 
   async ngOnInit(): Promise<void> {
     await this.load();
@@ -102,6 +106,7 @@ export class AdminCategoriesComponent implements OnInit {
     this.editing.set(null);
     this.deleting.set(null);
     this.deleteError.set(null);
+    this.formError.set(null);
     this.deleteOpen.set(false);
     this.modalMode.set('create');
   }
@@ -110,6 +115,8 @@ export class AdminCategoriesComponent implements OnInit {
     this.deleting.set(null);
     this.deleteOpen.set(false);
     this.editing.set(c);
+    this.deleteError.set(null);
+    this.formError.set(null);
     this.modalMode.set('edit');
   }
 
@@ -127,9 +134,13 @@ export class AdminCategoriesComponent implements OnInit {
     this.editing.set(null);
     this.deleting.set(null);
     this.deleteError.set(null);
+    this.formError.set(null);
+    this.saving.set(false);
   }
 
   async onFormSubmit(payload: CategoryFormSubmit): Promise<void> {
+    this.saving.set(true);
+    this.formError.set(null);
     try {
       if (payload.mode === 'create') {
         const created = await this.admin.createCategory({
@@ -151,6 +162,7 @@ export class AdminCategoriesComponent implements OnInit {
         }
         await this.admin.updateCategory(id, {
           name: payload.name,
+          abbreviation: payload.abbreviation,
           description: payload.description,
           iconPath,
         });
@@ -158,7 +170,9 @@ export class AdminCategoriesComponent implements OnInit {
       this.closeModal();
       await this.load();
     } catch (e: any) {
-      this.deleteError.set(e?.error?.message ?? e?.message ?? 'Failed');
+      this.formError.set(e?.error?.message ?? e?.message ?? 'Failed');
+    } finally {
+      this.saving.set(false);
     }
   }
 

frontend/src/app/features/admin/categories/category-form-modal.component.ts

@@ -92,9 +92,13 @@ export function syncCategoryForm(
             }
           </div>
 
+          @if (errorMessage()) {
+            <p class="error" data-testid="cf-error" style="color: var(--color-danger, #f00); margin-top: 8px;">{{ errorMessage() }}</p>
+          }
+
           <div class="actions">
             <button type="button" (click)="onCancel()" data-testid="cf-cancel">Cancel</button>
-            <button type="button" [disabled]="form.invalid || submitting()" (click)="onOk()" data-testid="cf-ok">OK</button>
+            <button type="button" [disabled]="form.invalid || submitting() || saving()" (click)="onOk()" data-testid="cf-ok">OK</button>
           </div>
         </form>
       </div>
@@ -112,6 +116,8 @@ export class CategoryFormModalComponent {
 
   readonly cancel = output<void>();
   readonly submit = output<CategoryFormSubmit>();
+  readonly errorMessage = input<string | null>(null);
+  readonly saving = input(false);
 
   readonly iconPreview = signal<string | null>(null);
   readonly iconFile = signal<File | null>(null);

tests/backend/uploads.spec.ts

@@ -6,6 +6,7 @@ process.env.UPLOAD_SIZE_LIMIT = '1mb';
 
 import * as fs from 'fs';
 import * as path from 'path';
+import sharp from 'sharp';
 
 import { safeFilename, parseUploadSizeLimit } from '../../backend/src/common/utils/upload';
 
@@ -124,22 +125,75 @@ describe('Uploads endpoint integration', () => {
     expect([401, 403]).toContain(res.status);
   });
 
-  it('uploads a small file successfully', async () => {
+  it('uploads a valid image and writes a deterministic 128x128 PNG', async () => {
+    const { agent, csrf } = await primeCsrf();
+    const png = await sharp({
+      create: { width: 64, height: 64, channels: 3, background: { r: 1, g: 2, b: 3 } },
+    }).png().toBuffer();
+    const res = await agent
+      .post('/api/v1/uploads/category-icon')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .set('X-CSRF-Token', csrf)
+      .field('categoryId', 'icon-test-cat')
+      .attach('file', png, 'icon.png');
+    expect(res.status).toBe(201);
+    expect(res.body.id).toBe('icon-test-cat.png');
+    expect(res.body.publicUrl).toBe('/uploads/icons/icon-test-cat.png');
+    expect(res.body.width).toBe(128);
+    expect(res.body.height).toBe(128);
+    expect(res.body.mimeType).toBe('image/png');
+    const written = path.join(process.env.UPLOAD_DIR!, 'icons', 'icon-test-cat.png');
+    expect(fs.existsSync(written)).toBe(true);
+    const decoded = await sharp(written).metadata();
+    expect(decoded.format).toBe('png');
+    expect(decoded.width).toBe(128);
+    expect(decoded.height).toBe(128);
+    await request(app.getHttpServer()).get(res.body.publicUrl).expect(200);
+  });
+
+  it('rejects a plain-text payload labeled as a category icon and does not overwrite a valid icon', async () => {
+    const iconsDir = path.join(process.env.UPLOAD_DIR!, 'icons');
+    fs.mkdirSync(iconsDir, { recursive: true });
+    const target = path.join(iconsDir, 'icon-tx-cat.png');
+    const validPng = await sharp({
+      create: { width: 32, height: 32, channels: 3, background: { r: 9, g: 9, b: 9 } },
+    }).png().toBuffer();
+    fs.writeFileSync(target, validPng);
+    const beforeBytes = fs.readFileSync(target);
+
     const { agent, csrf } = await primeCsrf();
     const res = await agent
       .post('/api/v1/uploads/category-icon')
       .set('Authorization', `Bearer ${adminToken}`)
       .set('X-CSRF-Token', csrf)
-      .attach('file', Buffer.from('hello world'), 'icon.png');
-    expect(res.status).toBe(201);
-    expect(res.body.id).toMatch(/^icon-[0-9a-f]{8}\.png$/);
-    expect(res.body.publicUrl).toBe(`/uploads/icons/${res.body.id}`);
-    // File actually written under UPLOAD_DIR/icons
-    const written = path.join(process.env.UPLOAD_DIR!, 'icons', res.body.id);
-    expect(fs.existsSync(written)).toBe(true);
-    // Public-served via /uploads static handler.
-    const fetched = await request(app.getHttpServer()).get(res.body.publicUrl).expect(200);
-    expect(fetched.body?.toString()).toBe('hello world');
+      .field('categoryId', 'icon-tx-cat')
+      .attach('file', Buffer.from('not an image'), { filename: 'icon.png', contentType: 'image/png' });
+    expect(res.status).toBe(400);
+    expect(res.body?.message).toMatch(/valid PNG|JPEG|GIF|WebP/i);
+    expect(fs.existsSync(target)).toBe(true);
+    expect(fs.readFileSync(target).equals(beforeBytes)).toBe(true);
+  });
+
+  it('rejects a corrupt PNG payload and does not overwrite a valid icon', async () => {
+    const iconsDir = path.join(process.env.UPLOAD_DIR!, 'icons');
+    const target = path.join(iconsDir, 'icon-corrupt-cat.png');
+    const validPng = await sharp({
+      create: { width: 32, height: 32, channels: 3, background: { r: 5, g: 5, b: 5 } },
+    }).png().toBuffer();
+    fs.writeFileSync(target, validPng);
+    const beforeBytes = fs.readFileSync(target);
+
+    const { agent, csrf } = await primeCsrf();
+    const corrupt = Buffer.concat([Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), Buffer.alloc(53, 0x00)]);
+    const res = await agent
+      .post('/api/v1/uploads/category-icon')
+      .set('Authorization', `Bearer ${adminToken}`)
+      .set('X-CSRF-Token', csrf)
+      .field('categoryId', 'icon-corrupt-cat')
+      .attach('file', corrupt, { filename: 'corrupt.png', contentType: 'image/png' });
+    expect(res.status).toBe(400);
+    expect(res.body?.message).toMatch(/valid PNG|JPEG|GIF|WebP/i);
+    expect(fs.readFileSync(target).equals(beforeBytes)).toBe(true);
   });
 
   it('rejects oversize uploads (limit=1mb, file=2mb)', async () => {

tests/frontend/admin-categories-form-modal.spec.ts

@@ -3,6 +3,7 @@ import { FormBuilder, FormControl, FormGroup } from '@angular/forms';
 import {
   syncCategoryForm,
   CategoryFormMode,
+  CategoryFormSubmit,
 } from '../../frontend/src/app/features/admin/categories/category-form-modal.component';
 import { AdminCategory } from '../../frontend/src/app/core/services/admin.service';
 
@@ -185,4 +186,36 @@ describe('CategoryFormModalComponent - prefill contract', () => {
     expect(form.controls.abbreviation.value).toBe('HW');
     expect(form.controls.description.value).toBe('Hardware challenges');
   });
+
+  it('keeps user (non-system) abbreviation editable and emits the uppercased abbreviation', () => {
+    const form = buildForm();
+    const result = syncCategoryForm(
+      form,
+      'edit',
+      makeRow({
+        id: 'user-cat',
+        name: 'Cat A Test T007',
+        abbreviation: 'CATA',
+        description: 'd',
+        isSystem: false,
+        systemKey: null,
+      }),
+    );
+    expect(result.abbreviationReadonly).toBe(false);
+
+    form.controls.name.setValue('Cat Z Renamed T007');
+    form.controls.abbreviation.setValue('zzza');
+    form.controls.description.setValue('Renamed to ZZZA for T007');
+
+    const v = form.getRawValue();
+    const emitted: CategoryFormSubmit = {
+      mode: 'edit',
+      name: v.name,
+      abbreviation: v.abbreviation.toUpperCase(),
+      description: v.description,
+    };
+    expect(emitted.abbreviation).toBe('ZZZA');
+    expect(emitted.name).toBe('Cat Z Renamed T007');
+    expect(emitted.description).toBe('Renamed to ZZZA for T007');
+  });
 });