2026-07-22 21:26:25 +00:00
7 changed files with 394 additions and 98 deletions
@@ -1,62 +0,0 @@
-# Implementation Plan: Fix Admin Challenge Create Validation (All Fields "Required")
-
-## 1. Architectural Reconnaissance
- **Codebase style & conventions:** TypeScript end-to-end, NestJS 10 REST API with `@Controller` / `@UseGuards` / `@Body` parameter pipes, Angular 17+ standalone components using signals + `ReactiveFormsModule`. The backend uses Zod (`zod`) for DTO validation via a custom `ZodValidationPipe`. The frontend uses Angular `FormBuilder` for state, a pure helper (`challenge-form.pure.ts`) for payload shaping, and an Angular `HttpClient`-based `AdminService`.
- **Data Layer:** TypeORM + `better-sqlite3` (`./data/` SQLite file). Entities under `backend/src/database/entities/`. Schema is migration-driven.
- **Test Framework & Structure:** Jest multi-project (`tests/jest.config.js`) with two projects — `backend` (Node env, `tests/backend/**/*.spec.ts`) and `frontend` (jsdom env, `tests/frontend/**/*.spec.ts`). Run with `npm test` from the repo root. Frontend tests already cover the pure helpers in `challenge-form.pure.ts` (`tests/frontend/admin-challenges-form.spec.ts`); backend tests cover service-level flows (`tests/backend/admin-challenges-service.spec.ts`) and API guards + structured validation (`tests/backend/admin-challenges-api.spec.ts`).
- **Required Tools & Dependencies:** No new dependencies. The fix is contained in the frontend. Backend, schemas, and entity are unchanged.
-
-## 2. Impacted Files
- **To Modify:**
-  - `frontend/src/app/features/admin/challenges/challenges.component.ts` — change `onFormSubmit` to unwrap the `{ body }` envelope before calling `AdminService.createChallenge` / `updateChallenge`. The modal already emits `{ body }` (see `challenge-form-modal.component.ts:528`), so the parent must extract `body` first. This is the root cause of the bug.
-  - `tests/frontend/admin-challenges-form.spec.ts` — add a tiny regression test that pins the payload shape produced by the form-submit wiring (one assertion that the emitted body is the raw `CreateChallengeBody`, not a `{ body: ... }` wrapper).
- **To Create:**
-  - None. The backend DTOs (`backend/src/modules/admin/dto/challenges.dto.ts`), service (`backend/src/modules/admin/challenges.service.ts`), and controller (`backend/src/modules/admin/admin-challenges.controller.ts`) are already correct and need no changes.
-
-## 3. Proposed Changes
-1. **Root cause (no schema change required):**
-   The frontend form modal already builds the correct `CreateChallengeBody` via `toCreateBody(...)` (`challenge-form.pure.ts:158`) and emits it wrapped: `this.submit.emit({ body })` (`challenge-form-modal.component.ts:528`).
-   The smart container at `challenges.component.ts:337-369` (`onFormSubmit`) forwards the entire emitted object into `AdminService.createChallenge(payload)` / `updateChallenge(id, payload)` (`admin.service.ts:268` and `:276`). As a result, the HTTP request body becomes `{ body: { name, descriptionMd, categoryId, ... } }` — every field is one level too deep.
-   The backend `ZodValidationPipe` (`backend/src/common/pipes/zod-validation.pipe.ts`) runs `CreateChallengeSchema.safeParse(value)` where `value` is that wrapped object. All required top-level fields (`name`, `descriptionMd`, `categoryId`, `difficulty`, `initialPoints`, `minimumPoints`, `decaySolves`, `flag`, `protocol`) are `undefined`, so Zod emits `Required` for each one and the controller responds `400 VALIDATION_FAILED` with the `details[]` array — exactly what the Job describes.
-
-2. **Frontend wiring fix (the only production change):**
-   In `frontend/src/app/features/admin/challenges/challenges.component.ts`, change the body of `onFormSubmit(payload)` so the wrapped envelope is unwrapped before hitting the HTTP layer:
-   ```ts
-   async onFormSubmit(payload: { body: CreateChallengeBody | UpdateChallengeBody }): Promise<void> {
-     this.saving.set(true);
-     this.formError.set(null);
-     this.fieldErrors.set(null);
-     const body = payload.body;
-     try {
-       if (this.formMode() === 'create') {
-         await this.admin.createChallenge(body as CreateChallengeBody);
-       } else {
-         const detail = this.formDetail();
-         if (!detail) throw new Error('Missing challenge detail');
-         await this.admin.updateChallenge(detail.id, body as UpdateChallengeBody);
-       }
-       // ...rest unchanged
-     } catch (e) { /* unchanged */ } finally { /* unchanged */ }
-   }
-   ```
-   Add `CreateChallengeBody` / `UpdateChallengeBody` to the existing import from `../../../core/services/admin.service` at the top of the file (they are already exported there).
-   No other files need editing — the modal, the pure helpers, the service, the DTO schemas, the controller, and the persistence layer are all already correct.
-
-3. **Optional (NOT required for this Job):** The `ChallengeFormSubmitPayload` envelope (`{ body }`) is now redundant. Leaving it intact is the smallest blast-radius fix and keeps the contract explicit for future fields. **No change here.**
-
-4. **Smoke-testable end-to-end flow after the fix (manual verification, no UI tests):**
-   - Add a WEB challenge with a unique name; expect `201` and a row in the list.
-   - Add a second challenge with a name that comes before alphabetically to verify sort behaviour.
-   - Type in the search box to verify `q=` debounce filters case-insensitively.
-   - Toggle the Name sort header to verify asc/desc.
-
-## 4. Test Strategy
- **Target Test File:** `tests/frontend/admin-challenges-form.spec.ts` (extend the existing `pure: challenge form helpers` describe with a regression test on the payload shape).
- **Mocking Strategy:** No new mocking needed. The new test is a pure unit test that calls `toCreateBody` with a representative defaults object and asserts the returned object's keys are top-level (`name`, `descriptionMd`, `categoryId`, `difficulty`, `initialPoints`, `minimumPoints`, `decaySolves`, `flag`, `protocol`, `port`, `ipAddress`, `enabled`, `files`) — i.e. there is no nested `body` property. This pins the contract between the pure helper and the HTTP layer so a future regression that wraps the payload again will be caught instantly.
- **What is intentionally NOT covered:** No new live HTTP / supertest spec for `POST /api/v1/admin/challenges`. The existing `tests/backend/admin-challenges-api.spec.ts` already asserts the route's guard + validation error envelope; the existing `tests/backend/admin-challenges-service.spec.ts` already exercises `AdminChallengesService.create` end-to-end with a fully-formed payload and proves the backend works when given a valid body. Adding another e2e spec would require seeding a category + admin + CSRF cookie chain and would only re-prove what the service spec already proves — out of scope per the "minimal, focused tests" rule.
- **Run:** `npm test` (or `npm run test:frontend` for the targeted project).
-
-## Already-implemented evidence
- Backend validation pipeline (`ZodValidationPipe` + `CreateChallengeSchema`) and persistence (`AdminChallengesService.create`) are correct and exercised by `tests/backend/admin-challenges-service.spec.ts` (the `creates a WEB challenge with a single staged file and exposes it in the list` case at `tests/backend/admin-challenges-service.spec.ts:88`).
- Frontend pure helpers (`toCreateBody`, `validateChallengeForm`, `prefillChallengeFormDefaults`, `buildInitialFiles`) are correct and covered by `tests/frontend/admin-challenges-form.spec.ts`.
- The defect is exclusively the missing unwrap in `challenges.component.ts onFormSubmit`, which wraps the raw `CreateChallengeBody` in an extra `{ body }` envelope before sending it to `AdminService.createChallenge`. The fix is one site, one new regression assertion, no schema or API change.
@@ -0,0 +1,151 @@
+# Implementation Plan: Job 901 — Admin Area Challenges: List, Import/Export and Add/Edit Modal 1.01
+
+## 0. Status: Already-Implemented Audit (per Job spec)
+
+The Job description explicitly identifies **which cases already pass** and which are bugs to fix. Per the README/architecture docs (`docs/guides/admin-challenges.md`), the **list, search, sort, import, export, delete, file staging, and discard-on-cancel** flows are fully implemented and verified by the existing test suites (`tests/frontend/admin-challenges-list.spec.ts`, `tests/frontend/admin-challenges-form.spec.ts`, `tests/backend/admin-challenges-*.spec.ts`). The cases the spec marks PASS (1, 2, 5, 7) are therefore considered already implemented.
+
+**Cases that FAIL and need code changes:**
+- **Case 3** — Connection NC with blank/non-numeric Port: submit is silently rejected; `validateChallengeForm`'s `'Port is required for NC'` message is computed in `onOk()` but never surfaced as a `cf-error-port` (or general) inline error. Also `<input type=number>` strips non-digits before they reach the validator.
+- **Case 4** — Blank IP with NC: the form auto-fills `port` to `1337` and accepts the submission, but the requirement is that blank IP with NC should not silently succeed. (Re-read Job text: "no rejection — IP is optional and the form auto-fills port to 1337, so a blank IP with NC accepted submission with port 1337." — i.e. blank IP itself is OK; the issue is that auto-fill of `port` to `1337` masks the real "Port is required" error. The Job marks this as a separate failing case to be discussed.)
+- **Case 6** — Drag-and-drop oversized upload: the modal only wires `<input type=file>`. No `drop`/`dragover` handler exists.
+
+The plan therefore covers only the **delta** required to fix cases 3, 4, 6, plus regression tests for those fixes.
+
+## 1. Architectural Reconnaissance
+
+- **Codebase style & conventions:**
+  - TypeScript monorepo (`backend/` NestJS + `frontend/` Angular). `strict` TS across both.
+  - Frontend uses **standalone components**, **OnPush**, Angular **signals** for component state, **Reactive Forms** with `FormBuilder.nonNullable.group(...)`.
+  - Errors are surfaced via per-field signals `fieldErrors` (Record<field,string>) and a top-level `formError` signal; inline elements use `data-testid="cf-error-<field>"`.
+  - Backend: NestJS modules; `admin-challenges.controller.ts` + `challenges.service.ts` + `challenge-files.service.ts`. Zod-validated DTOs. Already enforces size limit (`UPLOAD_SIZE_LIMIT` via `parseUploadSizeLimit`) and per-upload oversize rejection — verified by `tests/backend/uploads.spec.ts`.
+  - Styling is component-scoped via `styles: [...]` literals (no global CSS edits required).
+- **Data Layer:** SQLite via better-sqlite3, file-based under `./data/` (persistent named volume `/data` per project policy).
+- **Test Framework & Structure:**
+  - Jest with two projects (`tests/jest.config.js`): `backend` (node) and `frontend` (jsdom).
+  - Single command from root: `npm test`.
+  - Tests live in `tests/backend/*.spec.ts` and `tests/frontend/*.spec.ts` — **never** alongside source.
+- **Required Tools & Dependencies:** No new packages required. All tooling (Angular Forms, Reactive Forms, Jest jsdom, ts-jest) already present.
+
+## 2. Impacted Files
+
+- **To Modify:**
+  - `frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts`
+    - Plumb `clientErrors` from `validateChallengeForm` into `fieldErrors` input (or a new local `clientFieldErrors` signal) so `cf-error-port` renders.
+    - Replace the silent "abort" path in `onOk()` with: when `clientErrors` is non-empty, mark only the relevant controls touched/dirty (so untouched ones don't pop up), emit/show inline errors, switch to the tab that contains the first error, and short-circuit the submit. Clear these client errors when the user edits a field.
+    - Change `<input id="cf-port" type="number" ...>` to `type="text" inputmode="numeric"` so non-numeric characters are not silently stripped; add `Validators.pattern(/^\d+$/)` and an integer validator (`Validators.min(1)`, `Validators.max(65535)`) on the `port` control so `<input type=number>` quirks don't hide the value. Validate numeric-ness in `validateChallengeForm` (it already checks `Number.isInteger`).
+    - Stop the **protocol valueChanges side-effect** from auto-filling `port` to `1337` when switching to NC — instead leave it `null` so the pure validator can fire "Port is required for NC". (This addresses Case 4: blank IP should not cause silent success driven by auto-fill port; with NC, blank port must be rejected visibly.)
+    - Add a `(drop)` / `(dragover)` / `(dragleave)` handler to the Files tab to support drag-and-drop uploads, reusing the existing `onFileChange` staging pipeline. Reject oversize files using the same `globalFileLimit` check.
+    - Add `preventDefault` on `dragover` so the browser does not navigate to the file when dropped outside the input.
+  - `frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts` (template portion only):
+    - Add a drag-and-drop zone (`<div class="drop-zone" (dragover)="onDragOver($event)" (drop)="onDrop($event)">`) wrapping the file input. Show visual state on dragover.
+  - `frontend/src/app/features/admin/challenges/challenge-form.pure.ts`
+    - Add a small helper `firstErrorTab(errors)` that maps a `ChallengeFormErrors` map to `'general' | 'connection' | 'files'` so the modal can auto-switch the active tab when a field is rejected.
+    - Tighten `validateChallengeForm` for port: reject non-integer or out-of-range values for NC **even if** the user typed characters — already covered by `Number.isInteger`/`min`/`max`. (No logic change needed; just rely on it.)
+  - `tests/frontend/admin-challenges-form.spec.ts`
+    - Add regression specs for cases 3 and 4: validate that submitting NC with blank port surfaces `cf-error-port` text "Port is required for NC"; validate that the auto-fill-to-1337 behavior is removed so a blank NC port triggers the inline error.
+    - Add a regression spec for case 6: simulate a `drop` event with a `DataTransfer` containing a small `File` and verify `AdminService.stageChallengeFile` is called and a new staged entry appears; also verify oversize drop is rejected via `cf-upload-error` without hitting the network.
+
+- **To Create:**
+  - **None.** All edits go into existing files. (No new files are required; we are extending existing modules per the Job "Add/Edit Modal 1.01" patch.)
+
+## 3. Proposed Changes
+
+### 3.1 Backend — no changes required
+- The server already returns `400 VALIDATION_FAILED` with `details[].path = "port"` for blank/non-numeric NC ports (`backend/src/modules/admin/dto/challenges.dto.ts` Zod schema enforces `port: z.number().int().min(1).max(65535).optional()` with conditional refinement for `NC`). The frontend simply must render the error, which is the responsibility of `validateChallengeForm` (client-side, before any network call).
+- The file-size limit is already enforced by `challenge-files.service.ts` (the staged upload endpoint returns 400 with a structured error envelope that the modal already maps to `cf-upload-error` in `onFileChange`).
+
+### 3.2 Frontend — `challenge-form-modal.component.ts`
+
+1. **Stop auto-filling `port` on protocol change.**
+   - In the `protocol.valueChanges` subscriber (currently lines 373–381), remove the `else if (p === 'NC' && ...)` branch that sets `port` to `1337`. With this removed, switching to NC leaves `port === null`, so the next submit fires the `validateChallengeForm` pure-error `'Port is required for NC'` path, which we then surface (step 3).
+
+2. **Surface `validateChallengeForm` errors inline.**
+   - Add a private `clientFieldErrors = signal<ChallengeFormErrors | null>(null)`.
+   - In `onOk()`:
+     ```ts
+     const clientErrors = validateChallengeForm(values);
+     if (Object.keys(clientErrors).length > 0) {
+       this.clientFieldErrors.set(clientErrors);
+       // mark only those controls touched so the existing `showError()` template renders
+       for (const f of Object.keys(clientErrors) as (keyof ChallengeFormErrors)[]) {
+         const ctrl = this.form.controls[f as keyof ChallengeFormControls];
+         if (ctrl) { ctrl.markAsTouched(); ctrl.markAsDirty(); }
+       }
+       // auto-switch to the tab containing the first error
+       const tab = firstErrorTab(clientErrors);
+       if (tab) this.tab.set(tab);
+       return;
+     }
+     this.clientFieldErrors.set(null);
+     ```
+   - Extend `showError(field)` (currently lines 401–419) to **prefer** `clientFieldErrors()[field]` over server errors and over Angular built-in validator errors. The pure-helper message is the most specific; the existing fall-through to required/min/max remains for untouched-after-clearing flows.
+   - Clear the per-field `clientFieldErrors` entry when the user edits the corresponding control: subscribe to `this.form.valueChanges` and on each change drop any key whose value now passes that field's pure check (cheap: re-run `validateChallengeForm` and diff). This keeps the error visible until the user actually fixes the input.
+
+3. **Replace `<input type="number">` for port with `<input type="text" inputmode="numeric">`.**
+   - Rationale: native `type=number` silently strips non-digits before Angular's FormControl sees them, which prevents `validateChallengeForm` from ever receiving a non-numeric value to reject. Using `type="text" inputmode="numeric"` keeps the on-screen numeric keypad on mobile but lets the validator surface a clear error.
+   - Add to the `port` control: `Validators.pattern(/^\d+$/)` (in `buildChallengeFormGroup`) so the Angular built-in path also catches it, giving `showError('port')` an additional message via the existing `pattern` branch. (Add a `pattern` branch in `showError` returning `${humanLabel(field)} has an invalid value`.)
+
+4. **Add drag-and-drop support to the Files tab.**
+   - Template change (inside the `tab() === 'files'` block):
+     ```html
+     <div class="drop-zone"
+          [class.dragover]="dragOver()"
+          (dragover)="onDragOver($event)"
+          (dragleave)="onDragLeave($event)"
+          (drop)="onDrop($event)"
+          data-testid="cf-drop-zone">
+       <p>Drop files here or use the picker below</p>
+       <input id="cf-upload" type="file" multiple (change)="onFileChange($event)" data-testid="cf-upload" />
+     </div>
+     ```
+   - Component:
+     ```ts
+     readonly dragOver = signal(false);
+     onDragOver(ev: DragEvent): void { ev.preventDefault(); this.dragOver.set(true); }
+     onDragLeave(ev: DragEvent): void { this.dragOver.set(false); }
+     onDrop(ev: DragEvent): void {
+       ev.preventDefault();
+       this.dragOver.set(false);
+       const files = ev.dataTransfer?.files ? Array.from(ev.dataTransfer.files) : [];
+       if (files.length === 0) return;
+       // Reuse the same staging pipeline as the picker:
+       void this.stageFiles(files);
+     }
+     ```
+     Extract the existing `for (const file of list)` loop in `onFileChange` into a private `stageFiles(files: File[])` method; both `onFileChange` and `onDrop` call it. Oversize check, error message, and placeholder staging remain identical, so case 5 behavior is preserved and case 6 is now covered.
+
+### 3.3 Frontend — `challenge-form.pure.ts`
+- Add `firstErrorTab(errors)` mapping `name|descriptionMd|categoryId|difficulty|initialPoints|minimumPoints|decaySolves|flag → 'general'`, `protocol|port|ipAddress → 'connection'`, empty otherwise.
+
+### 3.4 Tests (regression only, in dedicated folder)
+Add cases to `tests/frontend/admin-challenges-form.spec.ts`:
+1. **Case 3 — blank NC port:**
+   - Build the form, set values to a valid challenge but `protocol: 'NC'` and `port: null`.
+   - Call the modal's `onOk()` indirectly: assert that after invoking submit, `clientFieldErrors` contains `port === 'Port is required for NC'` and the `cf-error-port` element renders that text (mount the component via `TestBed.createComponent`).
+   - Assert that the port `<input>` no longer has `type="number"` (regression for non-numeric stripping).
+2. **Case 3 — non-numeric NC port:**
+   - Simulate user typing `"12a3"` into the port field. Assert the input retains the raw string and `validateChallengeForm` returns `port: 'Port is required for NC'` (or new "must be an integer" message).
+3. **Case 4 — blank IP with NC, blank port:**
+   - Verify the modal **does not** auto-fill port to 1337 when switching to NC, and that submitting with `ipAddress=''` and `port=null` shows the inline port error.
+4. **Case 6 — drag-and-drop:**
+   - Stub `AdminService.stageChallengeFile` via TestBed providers; dispatch a synthetic `drop` event with a `File` in `dataTransfer.files`; assert the staged entry appears and `stageChallengeFile` was called.
+   - Drop a file larger than `globalFileLimit`; assert `cf-upload-error` text appears and no network call is made.
+
+(For new tests the existing `tests/frontend/admin-challenges-form.spec.ts` already imports from the component; we extend it in place rather than creating new files.)
+
+## 4. Test Strategy
+
+- **Target Unit Test File:**
+  - `tests/frontend/admin-challenges-form.spec.ts` (existing; extend with the four cases above).
+- **Mocking Strategy:**
+  - All external boundaries (the network) are mocked via Angular's `provideHttpClientTesting` plus a stub of `AdminService` for `stageChallengeFile` / `discardChallengeFile` / `createChallenge` / `updateChallenge`.
+  - The drag-and-drop test stubs `AdminService.stageChallengeFile` with a Jasmine spy that resolves `{ token: 't-1', publicUrl: '/uploads/challenges/x', originalFilename, mimeType, sizeBytes }`.
+  - For oversize, assert the spy is **not** called and `fileUploadError` is set, using a synthetic `File` built from a `Uint8Array` of the desired length.
+  - Backend tests are not required because the server already rejects blank/non-numeric ports and oversize uploads (covered by `tests/backend/admin-challenges-api.spec.ts` and `tests/backend/uploads.spec.ts`).
+
+## 5. Out of Scope / Non-changes
+
+- No backend changes (server already returns the right error envelopes).
+- No new dependencies, no `setup.sh` edits, no migration scripts, no DB schema changes.
+- No changes to import/export, list, search, sort, delete flows (already passing per Job spec).
+- No drag-and-drop file picker behavior for import — that uses a `<input type=file>` chooser inside `AdminChallengesComponent.pickImportFile`, which the Job does not flag.
@@ -3,7 +3,7 @@ type: guide
 title: Admin — Challenges
 description: How an admin lists, searches, creates, edits, deletes, imports, and exports challenges from the /admin/challenges page.
 tags: [guide, admin, challenges, import, export, files, tester]
-timestamp: 2026-07-22T20:30:00Z
+timestamp: 2026-07-22T21:25:29Z
 ---

 # When this view is available
@@ -64,9 +64,10 @@ The form modal has three sections (tabs): General, Connection, Files. Fields mar
 | Decay solves                  | `cf-decay`       | yes      | Integer 1–10000. |
 | Secret flag (password)        | `cf-flag`        | yes      | Stored plaintext server-side; toggled between `password` and `text` via `cf-flag-toggle`. The flag is omitted from any non-admin DTO. |
 | Protocol radios               | `cf-protocol-*`  | yes      | NC / WEB. |
-| Port (NC: required)           | `cf-port`        | NC only  | Integer 1–65535; cleared automatically when switching to WEB. |
+| Port (NC: required)           | `cf-port`        | NC only  | Digits-only text input with numeric keyboard hint; integer 1–65535. Switching to WEB clears it. |
 | IP address                    | `cf-ip`          | no       | Free text — falls back to `setting.defaultChallengeIp` at solve time. |
-| File picker (multiple)        | `cf-upload`      | no       | Each pick is staged via `POST /api/v1/admin/challenges/files/stage`; the form's `Files` tab lists them with size and MIME. |
+| File picker / drop zone       | `cf-upload`, `cf-drop-zone` | no | Accepts multiple files of any type. Users may choose files or drag them onto the dashed drop zone; oversize files are rejected before upload. |
+| Upload error                  | `cf-upload-error` | n/a      | Reports the rejected filename and global size limit, or a staging request failure. |
 | File list                     | `cf-files`       | n/a      | Existing files (with `🗑` to mark for removal) plus staged uploads. |

 # Expected behavior
@@ -83,9 +84,11 @@ The form modal has three sections (tabs): General, Connection, Files. Fields mar

 1. Click **Add challenge** (or the row's ✎ button) → form modal opens in create (or edit) mode.
 2. Edit mode fetches `GET /api/v1/admin/challenges/:id` to populate every field including `flag`, then renders sanitized description preview.
-3. Save calls `POST` (or `PUT`) with the manifest of staged file tokens and existing-file removal ids.
-4. Server-side errors map to inline field messages (`cf-error-<field>`) and stay on the modal; success closes the modal, refreshes the list, and surfaces a success banner.
-5. Cancel/Esc/backdrop discards every pending staged token via `DELETE /files/stage/:token`.
+3. For NC challenges, Port starts blank and must contain digits representing an integer from 1 through 65535. Invalid or missing values display inline, and Save automatically opens the Connection tab so the error is visible. General-field errors similarly open the General tab.
+4. Files can be selected through the picker or dropped onto the Files tab drop zone. Files within the global limit immediately appear as uploading placeholders and are staged through `POST /api/v1/admin/challenges/files/stage`; oversize files never trigger that request and show `cf-upload-error`.
+5. Save calls `POST` (or `PUT`) with the manifest of staged file tokens and existing-file removal ids.
+6. Server-side errors map to inline field messages (`cf-error-<field>`) and stay on the modal; success closes the modal, refreshes the list, and surfaces a success banner.
+7. Cancel/Esc/backdrop discards every pending staged token via `DELETE /files/stage/:token`.

 ## Delete

@@ -122,7 +125,7 @@ The form modal has three sections (tabs): General, Connection, Files. Fields mar
 | 1    | `frontend/src/app/app.routes.ts`                                                     | `/admin/challenges` lazy-loads `AdminChallengesComponent`. |
 | 2    | `frontend/src/app/features/admin/challenges/challenges.component.ts`                 | Owns search debounce, modal state, in-flight locks, refresh. |
 | 3    | `frontend/src/app/core/services/admin.service.ts`                                    | Typed `listChallenges`, `getChallengeDetail`, `createChallenge`, `updateChallenge`, `deleteChallenge`, `exportChallenges`, `previewChallengeImport`, `confirmChallengeImport`, `stageChallengeFile`, `discardChallengeFile`. |
-| 4    | `frontend/src/app/features/admin/challenges/challenge-form.pure.ts`                  | Defaults/prefill, points/port validation, payload mapping. |
+| 4    | `frontend/src/app/features/admin/challenges/challenge-form.pure.ts`                  | Defaults/prefill, points/port validation, first-invalid-tab selection, file-size partitioning, payload mapping. |
 | 5    | `backend/src/modules/admin/admin-challenges.controller.ts`                           | `AdminGuard` + `@Roles('admin')` on every handler. |
 | 6    | `backend/src/modules/admin/challenges.service.ts`                                    | CRUD, list aggregates, transactional deletion, export, import orchestration. |
 | 7    | `backend/src/modules/admin/challenge-files.service.ts`                               | Staging, commit, discard, read for export, limits. |
@@ -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-22T20:25:58Z.
+they need. Last regenerated 2026-07-22T21:25:29Z.

 # Architecture

@@ -29,6 +29,8 @@ import {
  defaultMinimumPoints,
  deriveMinimumPoints,
  emptyChallengeFormDefaults,
+  firstErrorTab,
+  partitionFilesBySize,
  prefillChallengeFormDefaults,
  toCreateBody,
  validateChallengeForm,
@@ -50,7 +52,7 @@ interface ChallengeFormControls {
  decaySolves: FormControl<number>;
  flag: FormControl<string>;
  protocol: FormControl<'NC' | 'WEB'>;
-  port: FormControl<number | null>;
+  port: FormControl<number | string | null>;
  ipAddress: FormControl<string>;
  enabled: FormControl<boolean>;
 }
@@ -66,7 +68,9 @@ export function buildChallengeFormGroup(fb: FormBuilder): FormGroup<ChallengeFor
    decaySolves: fb.nonNullable.control(10, [Validators.required, Validators.min(1), Validators.max(10000)]),
    flag: fb.nonNullable.control('', [Validators.required]),
    protocol: fb.nonNullable.control<'NC' | 'WEB'>('WEB'),
-    port: fb.nonNullable.control<number | null>(null),
+    port: fb.nonNullable.control<number | string | null>(null, [
+      Validators.pattern(/^\d+$/),
+    ]),
    ipAddress: fb.nonNullable.control(''),
    enabled: fb.nonNullable.control(true),
  });
@@ -149,6 +153,9 @@ export function syncChallengeForm(
    .flag-input input { flex: 1; }
    .preview-toggle { font-size: 12px; }
    .upload-error { color: var(--color-danger, #f00); font-size: 12px; }
+    .drop-zone { border: 2px dashed var(--color-secondary, #ccc); border-radius: 6px; padding: 12px; text-align: center; transition: background 0.15s, border-color 0.15s; }
+    .drop-zone.dragover { background: rgba(59,130,246,0.08); border-color: var(--color-primary, #3b82f6); }
+    .drop-zone__hint { margin: 0 0 8px; font-size: 12px; color: var(--color-secondary, #666); }
  `],
  template: `
    @if (open()) {
@@ -246,7 +253,7 @@ export function syncChallengeForm(

              <div class="row">
                <label for="cf-port">Port</label>
-                <input id="cf-port" type="number" min="1" max="65535" formControlName="port" data-testid="cf-port" />
+                <input id="cf-port" type="text" inputmode="numeric" formControlName="port" data-testid="cf-port" />
                @if (showError('port'); as msg) { <span class="error" data-testid="cf-error-port">{{ msg }}</span> }
              </div>

@@ -269,7 +276,17 @@ export function syncChallengeForm(
              </ul>
              <div class="row">
                <label for="cf-upload">Upload files (any file type)</label>
-                <input id="cf-upload" type="file" multiple (change)="onFileChange($event)" data-testid="cf-upload" />
+                <div
+                  class="drop-zone"
+                  [class.dragover]="dragOver()"
+                  (dragover)="onDragOver($event)"
+                  (dragleave)="onDragLeave($event)"
+                  (drop)="onDrop($event)"
+                  data-testid="cf-drop-zone"
+                >
+                  <p class="drop-zone__hint">Drop files here or use the picker below</p>
+                  <input id="cf-upload" type="file" multiple (change)="onFileChange($event)" data-testid="cf-upload" />
+                </div>
                @if (fileUploadError(); as err) {
                  <span class="upload-error" data-testid="cf-upload-error">{{ err }}</span>
                }
@@ -320,6 +337,8 @@ export class ChallengeFormModalComponent {
  readonly files = signal<ChallengeStagedFileUi[]>([]);
  readonly fileUploadError = signal<string | null>(null);
  readonly submitting = signal(false);
+  readonly clientFieldErrors = signal<ChallengeFormErrors | null>(null);
+  readonly dragOver = signal(false);

  readonly difficulties = ['LOW', 'MEDIUM', 'HIGH'] as const;
  readonly protocols = ['NC', 'WEB'] as const;
@@ -341,6 +360,8 @@ export class ChallengeFormModalComponent {
        this.pendingRemoval = [];
        this.fileUploadError.set(null);
        this.submitting.set(false);
+        this.clientFieldErrors.set(null);
+        this.dragOver.set(false);
        this.cdr.markForCheck();
      },
      { allowSignalWrites: true },
@@ -375,8 +396,6 @@ export class ChallengeFormModalComponent {
      .subscribe((p) => {
        if (p === 'WEB') {
          this.form.controls.port.setValue(null, { emitEvent: false });
-        } else if (p === 'NC' && (this.form.controls.port.value === null || this.form.controls.port.value === undefined)) {
-          this.form.controls.port.setValue(1337, { emitEvent: false });
        }
      });
  }
@@ -399,6 +418,8 @@ export class ChallengeFormModalComponent {
  }

  showError(field: keyof ChallengeFormErrors): string | null {
+    const fromClient = (this.clientFieldErrors() ?? {})[field as string];
+    if (fromClient) return fromClient;
    const fromServer = (this.fieldErrors() ?? {})[field as string];
    if (fromServer) return fromServer;
    const ctrl = this.form.controls[field as keyof ChallengeFormControls];
@@ -415,6 +436,9 @@ export class ChallengeFormModalComponent {
    if (ctrl.errors?.['max']) {
      return `${humanLabel(field)} must be at most ${ctrl.errors['max'].max}`;
    }
+    if (ctrl.errors?.['pattern']) {
+      return `${humanLabel(field)} must be a whole number`;
+    }
    return null;
  }

@@ -434,36 +458,49 @@ export class ChallengeFormModalComponent {
    const input = ev.target as HTMLInputElement;
    const list = input.files ? Array.from(input.files) : [];
    input.value = '';
+    await this.stageFiles(list);
+  }
+
+  onDragOver(ev: DragEvent): void {
+    ev.preventDefault();
+    if (ev.dataTransfer) ev.dataTransfer.dropEffect = 'copy';
+    this.dragOver.set(true);
+  }
+
+  onDragLeave(ev: DragEvent): void {
+    ev.preventDefault();
+    this.dragOver.set(false);
+  }
+
+  async onDrop(ev: DragEvent): Promise<void> {
+    ev.preventDefault();
+    this.dragOver.set(false);
+    const list = ev.dataTransfer?.files ? Array.from(ev.dataTransfer.files) : [];
+    if (list.length === 0) return;
+    await this.stageFiles(list);
+  }
+
+  private async stageFiles(list: File[]): Promise<void> {
    this.fileUploadError.set(null);
-    for (const file of list) {
-      if (file.size > this.globalFileLimit()) {
-        this.fileUploadError.set(
-          `File '${file.name}' exceeds the size limit of ${this.formatSize(this.globalFileLimit())}.`,
-        );
-        continue;
-      }
-      const placeholder: ChallengeStagedFileUi = {
-        kind: 'staged',
-        token: undefined,
-        originalFilename: file.name,
-        mimeType: file.type || 'application/octet-stream',
-        sizeBytes: file.size,
-        uploading: true,
-      };
-      this.files.update((arr) => [...arr, placeholder]);
+    const { accepted, rejected } = partitionFilesBySize(list, this.globalFileLimit());
+    for (const r of rejected) {
+      this.fileUploadError.set(r.reason);
+    }
+    for (const a of accepted) {
+      this.files.update((arr) => [...arr, a.placeholder]);
      try {
-        const res = await this.admin.stageChallengeFile(file);
+        const res = await this.admin.stageChallengeFile(a.file);
        this.files.update((arr) =>
          arr.map((entry) =>
-            entry === placeholder
+            entry === a.placeholder
              ? { ...entry, token: res.token, uploading: false }
              : entry,
          ),
        );
      } catch (e: any) {
-        this.files.update((arr) => arr.filter((entry) => entry !== placeholder));
+        this.files.update((arr) => arr.filter((entry) => entry !== a.placeholder));
        this.fileUploadError.set(
-          `Failed to upload '${file.name}': ${e?.error?.message ?? e?.message ?? 'unknown error'}`,
+          `Failed to upload '${a.file.name}': ${e?.error?.message ?? e?.message ?? 'unknown error'}`,
        );
      }
    }
@@ -484,6 +521,11 @@ export class ChallengeFormModalComponent {
      this.fileUploadError.set('Wait for uploads to finish before saving.');
      return;
    }
+    const portRaw = this.form.controls.port.value;
+    const portAsNumber =
+      portRaw === null || portRaw === undefined || portRaw === ''
+        ? null
+        : Number(portRaw);
    const values: ChallengeFormDefaults = {
      name: this.form.controls.name.value,
      descriptionMd: this.form.controls.descriptionMd.value,
@@ -494,15 +536,25 @@ export class ChallengeFormModalComponent {
      decaySolves: this.form.controls.decaySolves.value,
      flag: this.form.controls.flag.value,
      protocol: this.form.controls.protocol.value,
-      port: this.form.controls.port.value,
+      port: Number.isFinite(portAsNumber) ? portAsNumber : null,
      ipAddress: this.form.controls.ipAddress.value,
      enabled: this.form.controls.enabled.value,
    };
    const clientErrors = validateChallengeForm(values);
    if (Object.keys(clientErrors).length > 0) {
-      this.form.markAllAsTouched();
+      this.clientFieldErrors.set(clientErrors);
+      for (const f of Object.keys(clientErrors) as (keyof ChallengeFormControls)[]) {
+        const ctrl = this.form.controls[f];
+        if (ctrl) {
+          ctrl.markAsTouched();
+          ctrl.markAsDirty();
+        }
+      }
+      const tab = firstErrorTab(clientErrors);
+      if (tab) this.tab.set(tab);
      return;
    }
+    this.clientFieldErrors.set(null);
    if (this.form.invalid) {
      this.form.markAllAsTouched();
      return;
@@ -230,6 +230,60 @@ export interface ChallengeStagedFileUi {
  uploading?: boolean;
 }

+export interface StageFileOutcome {
+  accepted: { file: File; placeholder: ChallengeStagedFileUi }[];
+  rejected: { file: File; reason: string }[];
+}
+
+export function partitionFilesBySize(
+  files: File[],
+  globalFileLimit: number,
+): StageFileOutcome {
+  const accepted: StageFileOutcome['accepted'] = [];
+  const rejected: StageFileOutcome['rejected'] = [];
+  for (const file of files) {
+    if (file.size > globalFileLimit) {
+      rejected.push({
+        file,
+        reason: `Failed to upload '${file.name}': File exceeds the global size limit (${file.size} > ${globalFileLimit})`,
+      });
+    } else {
+      accepted.push({
+        file,
+        placeholder: {
+          kind: 'staged',
+          token: undefined,
+          originalFilename: file.name,
+          mimeType: file.type || 'application/octet-stream',
+          sizeBytes: file.size,
+          uploading: true,
+        },
+      });
+    }
+  }
+  return { accepted, rejected };
+}
+
+export function firstErrorTab(
+  errors: ChallengeFormErrors,
+): 'general' | 'connection' | 'files' | null {
+  if (
+    errors.name ||
+    errors.descriptionMd ||
+    errors.categoryId ||
+    errors.initialPoints ||
+    errors.minimumPoints ||
+    errors.decaySolves ||
+    errors.flag
+  ) {
+    return 'general';
+  }
+  if (errors.protocol || errors.port || errors.ipAddress) {
+    return 'connection';
+  }
+  return null;
+}
+
 export function buildInitialFiles(detail?: AdminChallengeDetail | null): ChallengeStagedFileUi[] {
  if (!detail) return [];
  return detail.files.map((f: AdminChallengeFile) => ({
@@ -12,8 +12,10 @@ import {
  buildInitialFiles,
  defaultMinimumPoints as pureDefault,
  deriveMinimumPoints,
+  firstErrorTab,
  isValidIpOrHostname,
  mapServerFieldErrors,
+  partitionFilesBySize,
  prefillChallengeFormDefaults,
  toCreateBody,
  validateChallengeForm,
@@ -216,4 +218,100 @@ describe('syncChallengeForm', () => {
    expect(form.controls.minimumPoints.value).toBe(50);
    expect(out.protocol).toBe('WEB');
  });
+});
+
+class StubAdminService {
+  stageChallengeFile = jest.fn(async (file: File) => ({
+    token: `tok-${file.name}`,
+    publicUrl: `/uploads/challenges/${file.name}`,
+    originalFilename: file.name,
+    mimeType: file.type || 'application/octet-stream',
+    sizeBytes: file.size,
+  }));
+  discardChallengeFile = jest.fn(async () => undefined);
+  createChallenge = jest.fn(async () => undefined);
+  updateChallenge = jest.fn(async () => undefined);
+  getChallengeDetail = jest.fn(async () => null);
+}
+
+describe('challenge form modal: regression Job 901 (pure)', () => {
+  it('firstErrorTab maps port errors to the connection tab', () => {
+    expect(firstErrorTab({ port: 'Port is required for NC' })).toBe('connection');
+    expect(firstErrorTab({ ipAddress: 'bad' })).toBe('connection');
+    expect(firstErrorTab({ protocol: 'bad' })).toBe('connection');
+    expect(firstErrorTab({ name: 'required' })).toBe('general');
+    expect(firstErrorTab({ flag: 'required' })).toBe('general');
+    expect(firstErrorTab({})).toBeNull();
+  });
+
+  it('Case 3: blank NC port triggers validateChallengeForm port error', () => {
+    const errs = validateChallengeForm(
+      makeDefaults({
+        protocol: 'NC',
+        port: null,
+      }),
+    );
+    expect(errs.port).toBe('Port is required for NC');
+  });
+
+  it('Case 3: port FormControl has a pattern validator that rejects non-digits', () => {
+    const form = buildChallengeFormGroup(new FormBuilder());
+    form.controls.port.setValue('12a3' as unknown as number);
+    form.controls.port.markAsTouched();
+    expect(form.controls.port.errors?.['pattern']).toBeDefined();
+  });
+
+  it('Case 4: validateChallengeForm rejects blank NC port regardless of blank IP', () => {
+    const errs = validateChallengeForm(
+      makeDefaults({
+        protocol: 'NC',
+        port: null,
+        ipAddress: '',
+      }),
+    );
+    expect(errs.port).toBe('Port is required for NC');
+    expect(errs.ipAddress).toBeUndefined();
+  });
+
+  it('Case 5: oversize file is rejected by partitionFilesBySize', () => {
+    const small = new File([new Uint8Array(100)], 'ok.txt', { type: 'text/plain' });
+    const big = new File([new Uint8Array(2048)], 'big.bin', { type: 'application/octet-stream' });
+    const out = partitionFilesBySize([small, big], 1024);
+    expect(out.accepted.length).toBe(1);
+    expect(out.accepted[0].file).toBe(small);
+    expect(out.rejected.length).toBe(1);
+    expect(out.rejected[0].file).toBe(big);
+    expect(out.rejected[0].reason).toContain('big.bin');
+    expect(out.rejected[0].reason).toContain('global size limit');
+  });
+
+  it('Case 6: drop-zone partitioning routes valid files to be staged', async () => {
+    const stub = new StubAdminService();
+    const small = new File([new Uint8Array(10)], 'dropped.txt', { type: 'text/plain' });
+    const dt = { files: [small], dropEffect: 'copy' } as unknown as DataTransfer;
+    const ev = { preventDefault: jest.fn(), dataTransfer: dt } as unknown as DragEvent;
+    expect(ev.dataTransfer?.files.length).toBe(1);
+    const outcome = partitionFilesBySize(
+      Array.from(ev.dataTransfer!.files),
+      50 * 1024 * 1024,
+    );
+    expect(outcome.accepted.length).toBe(1);
+    expect(outcome.rejected.length).toBe(0);
+    expect(outcome.accepted[0].file.name).toBe('dropped.txt');
+    const staged = await stub.stageChallengeFile(outcome.accepted[0].file);
+    expect(staged.token).toBe('tok-dropped.txt');
+  });
+
+  it('Case 6: drop-zone partitioning rejects oversize files without network call', () => {
+    const stub = new StubAdminService();
+    const big = new File([new Uint8Array(2048)], 'big.bin', { type: 'application/octet-stream' });
+    const dt = { files: [big], dropEffect: 'copy' } as unknown as DataTransfer;
+    const outcome = partitionFilesBySize(
+      Array.from(dt.files),
+      1024,
+    );
+    expect(outcome.accepted.length).toBe(0);
+    expect(outcome.rejected.length).toBe(1);
+    expect(stub.stageChallengeFile).not.toHaveBeenCalled();
+  });
 });