diff --git a/.kilo/plans/904.md b/.kilo/plans/904.md deleted file mode 100644 index ece2616..0000000 --- a/.kilo/plans/904.md +++ /dev/null @@ -1,101 +0,0 @@ ---- -job: 904 -title: Challenges Page and Challenge Solve Modal Automation Readiness -status: investigation-complete ---- - -# Implementation Plan: Challenges Page and Challenge Solve Modal — Job 904 - -## 0. Pre-Implementation Status: **[ALREADY_IMPLEMENTED]** - -After tracing the entire `/challenges` flow end-to-end against the -docs (`docs/guides/challenges-board.md`, `docs/guides/event-window.md`) -and the source tree, every behavior the Job asks an admin/tester to -automate is **already present and stable**. The only defect reported -in the Job — *“Playwright tab, snapshot, and close operations timed out -after 30000ms”* — is an external automation-harness symptom (an idle -session after a logout), not a missing or broken feature in the SPA. - -**No source code or test edits are required to satisfy this Job.** -Only the missing fast-running automation-harness guard tests listed in -Section 4 are added, so that the SPA’s behaviour can be re-verified -headlessly with `npm test` in the future without needing a browser. - -### What already works (file references) - -| Behavior required by Job | Implemented in | -|---------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| Admin sets Stopped / Countdown / Running windows | `admin-general-event-window.spec.ts` + `AdminGeneralService` + `SettingsService` (settings table rows `eventStartUtc` / `eventEndUtc`). | -| Page renders a live `DD:HH:mm` countdown | `ChallengesPage.countdownText` computed (`challenges.page.ts:110-120`) using `formatDdHhMm` from `challenges.pure.ts`, anchored on `EventStatusStore.currentServerTimeUtc` (`event-status.store.ts:37-44`) driven by the 1-second local tick. | -| Single `window.location.reload()` at countdown zero | `ChallengesPage.ngOnInit` → `eventStatus.reloadAtCountdownZero(...)` (challenges.page.ts:150-152); the store fires the handler exactly once via `reloadOnZeroFired` latching (`event-status.store.ts:127-148`). | -| Stale-modal resume flow (modal stays mounted across data refresh) | Modal is selected via `selectedCard = signal(findCard(id))` (challenges.page.ts:122,162-175). `loadDetail` updates `selectedSolvers` only if `selectedCard()?.id === id` (challenges.page.ts:170-174) so a slow stale fetch can’t overwrite a newer modal. `clearSelectedDetail()` is invoked on close. | -| Flag submission rejected-then-accepted UX | `ChallengeModalComponent.onSubmit` (`challenge-modal.component.ts:179-201`): distinguishes `solved`, `already_solved`, `FLAG_INCORRECT`, and `EVENT_NOT_RUNNING` via `parseSolveError` / `messageForSolveError` in `challenges.pure.ts:150,206`; awards banner (`awarded` signal) + solvers list refresh + board card `solvedByMe` flip on success; second submit returns `already_solved` and shows the banner **plus** the inline error. | -| Stable automation hooks (data-testids) | `[data-testid="challenges-page"]`, `challenges-grid`, `challenges-gate`, `challenges-countdown`, `category-column-`, `challenge-card-`, `points-`, `challenge-modal-`, `modal-solved-banner`, `modal-awarded-banner`, `modal-error`, `flag-input`, `solve-button`, `modal-solvers`. | -| Clean teardown so logout does not leave a hung SSE source | `ChallengesStore.stop()` cancels the SSE source + interval (`challenges.store.ts:295-308`); `EventStatusStore.stop()` does the same on logout (`event-status.store.ts:150-167`); Home calls `clearAndBroadcast()` which closes transports and resets state. | - -Existing test coverage that proves the above at the harness level: - -- `tests/backend/challenges-board.spec.ts`, `challenges-status-rest.spec.ts`, `challenges-events-sse.spec.ts`, `challenges-submit-flag.spec.ts` -- `tests/frontend/challenges.pure.spec.ts`, `challenges.store.spec.ts`, `event-status.store.spec.ts`, `authenticated-event-source.spec.ts`, `notification-interceptor.spec.ts` - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** TypeScript end-to-end. NestJS backend (modules / controllers / services + DTO + class-validator). Angular 17 standalone SPA with `signal`/`computed` stores, `ChangeDetectionStrategy.OnPush`, `inject()`-based DI, `data-testid` hooks for the test harness. RxJS only for HTTP; SSE uses a custom `AuthenticatedEventSourceService` over `fetch` because the browser `EventSource` API cannot send the JWT header. -- **Data Layer:** SQLite (`better-sqlite3`) via a thin repository module. Time-related logic is pure (`EventStatusService.getState(now)`); the two `setting` rows (`eventStartUtc`, `eventEndUtc`) seeded by `SeedSystemData1700000000100`. No migrations needed for this Job. -- **Test Framework & Structure:** Jest with `ts-jest`, two projects (`backend` → node, `frontend` → jsdom). Root command: `npm test` (from `/repo`). Tests live exclusively under `/repo/tests/{backend,frontend}/*.spec.ts`. Frontend tests are logic-focused — no visual / browser assertions. `npm run dev` starts both processes; `/data` is the persistent named volume already mounted for uploads + sqlite journal files. No new persistent files are needed for these tests. -- **Required Tools & Dependencies:** None new. All required packages (`@angular/*`, `rxjs`, `jest`, `ts-jest`, `jest-environment-jsdom`, `@types/jest`, `@types/jsdom`, `jsdom`) are already in `package.json`. `setup.sh` already installs them and rebuilds `better-sqlite3`. Playwright is intentionally **not** added (matches the platform's “no UI test harness” rule and matches the failure mode described in the Job). - -## 2. Impacted Files - -- **To Modify:** None. The Behavior is already implemented. -- **To Create:** (small automation-readiness guard tests only — optional addition described in Section 4) - - `tests/frontend/challenges-page.spec.ts` — pure-logic regression for the gate/countdown/auto-reload wiring. - - `tests/frontend/challenge-modal-submit.spec.ts` — pure-logic regression for the rejected-then-accepted flag submit UX. - - These complement, not duplicate, the existing `challenges.store.spec.ts` and `challenges.pure.spec.ts`, by covering the **page** component (gate panel + auto-reload trigger) and the **modal** component (`onSubmit` branches + live-solve merge) which are currently only exercised indirectly. They use Angular `TestBed` against the same signal stores (mocking `ChallengesApiService` only), so they run in jsdom with no SSE server. - -## 3. Proposed Changes - -1. **Database / Schema Migration:** None required. -2. **Backend Logic & APIs:** None required. The four endpoints the Job’s tester needs (`GET /api/v1/challenges/board`, `GET /api/v1/challenges/:id`, `POST /api/v1/challenges/:id/solves`, `GET /api/v1/events`, `GET /api/v1/events/status`, plus `/api/v1/settings/event` for admin) are all in place and covered by `tests/backend/*`. -3. **Frontend UI Integration:** None required. The Challenges page is wired as documented: - - Gate vs grid is selected by `isRunning()` computed from `EventStatusStore.state` (`challenges.page.ts:50-72`). - - Auto-reload handler is registered in `ngOnInit` (idempotent) and re-armed on each `applyServerStatus` frame (`event-status.store.ts:81-91`). - - Challenge modal listens to SSE `solve` events via `registerSolveListener(id, ...)` and is torn down cleanly on close / destroy. - - Submit flow `solved` → `already_solved` is exercised by sending the same flag twice (the second call returns the `already_solved` envelope from the backend, which the modal maps to the inline error). - -### Concrete changes for the optional guard tests - -- `tests/frontend/chenges-page.spec.ts` - - Bootstrap `TestBed` with `provideHttpClientTesting()`, `provideRouter([])`, real `EventStatusStore`, fake `AuthenticatedEventSourceService` returning a stub `EventSourceLike`. - - Assert that when `EventStatusStore.state === 'unconfigured'` the page template renders `[data-testid="challenges-gate"]` and **not** `[data-testid="challenges-grid"]`. - - Drive `EventStatusStore.applyServerStatus({ state: 'running', … })` and assert the gate is replaced by the grid host div. - - Assert `EventStatusStore.reloadAtCountdownZero` was called exactly once during `ngOnInit` by passing a spy handler. - - Assert that `ChallengesStore.stop()` is invoked from the `DestroyRef` hook on `ngOnDestroy` (proves logout cleanly tears down the SSE source — directly addresses the Playwright hang reported in the Job). - -- `tests/frontend/challenge-modal-submit.spec.ts` - - Mount `ChallengeModalComponent` via `TestBed.createComponent`, inject a stub `ChallengesStore` whose `submit` resolves to a `SolveResponse` with `status: 'solved'`, then to `status: 'already_solved'`. - - Assert `errorMessage` is `null` after the first submit and `messageForSolveError('already_solved')` after the second; the `awarded` signal is set on both; the `submitted` event fires with the trimmed flag value; the flag input is cleared on success. - - Assert the `FLAG_INCORRECT` branch by rejecting with `{ status: 400, body: { code: 'FLAG_INCORRECT' } }` and verifying `errorMessage === 'Incorrect flag. Try again.'` and `submitting() === false` afterwards. - -Both files follow the existing patterns in `tests/frontend/admin-challenges-form.spec.ts` and `tests/frontend/challenges.store.spec.ts` (signal stubs, no real HTTP, no `BrowserAnimationsModule`). - -## 4. Test Strategy - -- **Target Unit Test Files:** - - `tests/frontend/challenges-page.spec.ts` - - `tests/frontend/challenge-modal-submit.spec.ts` -- **Mocking Strategy:** - - `ChallengesApiService` is replaced by an in-test `ChallengesApiService`-shaped object with `getBoard`, `getDetail`, `getEventState`, and `submit` returning `of(...)`. This avoids any HTTP call and lets the test resolve immediately in jsdom. - - `AuthenticatedEventSourceService` is replaced by a small class returning a minimal `EventSourceLike` stub (`addEventListener` / `close`). This is exactly what `tests/frontend/authenticated-event-source.spec.ts` already does for the SSE transport and avoids opening a real socket during teardown — which is what was hanging the previous Playwright session. - - Timer-based behaviour (`reloadAtCountdownZero`, the 1-second tick that drives the countdown) is exercised by setting `state = 'countdown'` with `secondsToStart = 0` directly on the store, so no fake timers are required. - - All tests are pure `TestBed`-based; no `BrowserAnimationsModule`, no `localStorage`/`BroadcastChannel` (the logout path is not the target of these tests — that is already covered by `tests/frontend/auth-session-events.spec.ts` and `tests/frontend/auth-restore.spec.ts`). -- **Running:** `npm test -- --testPathPattern=frontend/(challenges-page|challenge-modal-submit)` from `/repo` runs these in <1 second. No new dependencies, no UI, no changes to `setup.sh`. - -## 5. Acceptance Criteria (matches the Job) - -1. The Angular `/challenges` page renders the documented `data-testid` hook set (asserted by the new spec). -2. The SPA exposes a deterministic countdown-to-reload transition when the admin changes the event window to `countdown` → `running` → `stopped` (asserted by the existing `event-status.store.spec.ts` and the new `challenges-page.spec.ts`). -3. The challenge solve modal distinguishes `solved`, `already_solved`, and `FLAG_INCORRECT` and renders the correct inline error / awarded banner pair (asserted by the new `challenge-modal-submit.spec.ts` and the existing `challenges.pure.spec.ts`). -4. After logout, no SSE connection is left dangling: `DestroyRef`-driven teardown of `ChallengesStore` and `EventStatusStore` is asserted in the new page spec (covers the Playwright hang root cause). -5. `npm test` continues to run all suites (backend + frontend) in a single command from `/repo` with no new tools, ports, or browsers. diff --git a/.kilo/plans/905-handler-ownership.md b/.kilo/plans/905-handler-ownership.md new file mode 100644 index 0000000..7a733ed --- /dev/null +++ b/.kilo/plans/905-handler-ownership.md @@ -0,0 +1,124 @@ +# Implementation Plan: Decouple `reloadAtCountdownZero` from `start()`/`stop()` + +## Status +Hardening follow-up to Job 905. The store currently clears the +`reloadOnZero` handler inside `stop()` (line 197), and `start()` calls +`stop()` first (line 110). Because `ChallengesPage` registers its handler +from the constructor while `HomeComponent` calls `start()`/`stop()` +independently, a `stop()` triggered by the shell's lifecycle can null out +the page's handler before the page is destroyed. The fix is to scope +`reloadOnZero` ownership to the **subscriber** (the challenges page) +and make `start()`/`stop()` only manage the SSE source + tick, never the +reload handler. + +## 1. Architectural Reconventions (recap) +- `EventStatusStore` is a `providedIn: 'root'` singleton + (`frontend/src/app/core/services/event-status.store.ts`). +- `start(...)` is called from `HomeComponent.ngOnInit` + (`features/home/home.component.ts:132`); `stop()` is invoked from both + `ngOnDestroy()` and `destroyRef.onDestroy(...)` in that same component + (`features/home/home.component.ts:125, 142`). +- `ChallengesPage` currently registers its reload handler in its + constructor (`features/challenges/challenges.page.ts:128-133`). +- Tests live in `tests/frontend/*`, runnable via `npm test`. No new + dependencies; no backend changes. + +## 2. Impacted Files +- **To Modify:** + - `frontend/src/app/core/services/event-status.store.ts` — + separate handler ownership from SSE/timer ownership. + - `frontend/src/app/features/challenges/challenges.page.ts` — + own the subscription, unregister in `ngOnDestroy` (or via + `DestroyRef`). +- **To Create:** None. + +## 3. Proposed Changes + +### 3.1 `EventStatusStore` — split "transport lifecycle" from "reload subscription" + +1. Add a per-subscription API that returns an unregister function: + ```ts + subscribeReloadAtCountdownZero(handler: () => void): () => void { + this.reloadOnZero = handler; + this.reloadOnZeroFired = false; + this.ensureWatcher(); + return () => { + if (this.reloadOnZero === handler) { + this.reloadOnZero = null; + this.reloadOnZeroFired = false; + } + }; + } + ``` + Keep the existing `reloadAtCountdownZero(handler)` as a thin wrapper + that calls `subscribeReloadAtCountdownZero(handler)` and discards + the returned disposer (preserves backwards compatibility with the + four existing tests that call it directly). +2. In `start(...)`: remove the `this.stop()` call. `start()` should + only (a) close any **previous source** via a new private + `closeTransport()`, (b) open the new source, (c) install the message + handler, (d) start the tick interval, (e) `ensureWatcher()`. It + must NOT touch `reloadOnZero`, `reloadOnZeroFired`, or + `lastAppliedJson`. +3. Introduce `closeTransport()` (private) that nulls + `source`/`intervalId`/`zeroWatcher` ONLY — no handler reset. The + watcher interval is also bound to the transport lifecycle, but + `ensureWatcher()` keeps it idempotent, so closing it is fine. +4. In `stop()`: keep the current teardown for source + tick + watcher, + but **only clear `reloadOnZero`/`reloadOnZeroFired`/`lastAppliedJson` + when called via the destroy hook** (i.e., `stop()` retains its + existing full-reset semantics for end-of-life). To preserve that + for the existing destroyRef hook (`constructor() { destroyRef.onDestroy(() => this.stop()); }`), + keep `stop()` as the full reset. The crucial change is step 2: + `start()` no longer calls `stop()`, so handler state survives a + `start()` cycle. + +### 3.2 `ChallengesPage` — own the subscription + +1. Replace the constructor's + `this.eventStatus.reloadAtCountdownZero(...)` call with: + ```ts + const unsub = this.eventStatus.subscribeReloadAtCountdownZero(() => { + if (typeof window !== 'undefined') window.location.reload(); + }); + this.destroyRef.onDestroy(() => unsub()); + ``` + This keeps the subscription active across `start()` re-entry on the + store, registers the handler as early as possible (constructor), and + unregisters when the page is destroyed. +2. The existing `this.destroyRef.onDestroy(() => this.store.stop())` + for `ChallengesStore` is unrelated and stays. + +### 3.3 Tests + +1. **New spec** (`tests/frontend/event-status.store.watcher.spec.ts`, + add one case): `start()` after a `subscribeReloadAtCountdownZero` + must NOT clear the registered handler — fire a fake SSE delivery + that triggers `applyServerStatus` with a zero-countdown frame, then + advance fake timers and assert the handler still fires exactly once. +2. **Update** the existing + `tests/frontend/challenges-page.spec.ts` regression test + "repeating the same zero-countdown SSE frame does not disarm the + auto-reload latch (Job 905)" — no behaviour change required; it + already exercises `applyServerStatus` directly and continues to pass. +3. **New spec** for `subscribeReloadAtCountdownZero` unregister + semantics: subscribe → unregister → apply zero-countdown → advance + timers → handler must NOT be called. + +### 3.4 Documentation + +- Append one sentence to `docs/guides/event-window.md`'s "Auto-reload + at the transition boundary" paragraph noting that the subscription + is now page-owned and unregistered on destroy, independent of the + transport's `start()`/`stop()`. + +## 4. Test Strategy +- Single command: `npm test`. New assertions target + `tests/frontend/event-status.store.watcher.spec.ts` and use fake + timers (already used in this file). No mocking of `window.location` + is required — the handler under test is a plain `jest.fn()`. + +## 5. Non-goals +- No backend changes, no new dependencies, no `setup.sh` change. +- No changes to other consumers of `EventStatusStore` (HomeComponent's + `start()`/`stop()` cycle is preserved by step 3.1.2). \ No newline at end of file diff --git a/docs/architecture/key-files.md b/docs/architecture/key-files.md index 95c8e82..79beefc 100644 --- a/docs/architecture/key-files.md +++ b/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, the public bootstrap SSE listener, the challenges full-replace import flow, and the authenticated player-facing challenges board. tags: [architecture, key-files, event-window, validation, default-challenge-ip, sse, bootstrap, migrations, challenges, import, board, notifications] -timestamp: 2026-07-23T00:10:00Z +timestamp: 2026-07-23T01:19:00Z --- # Backend @@ -58,7 +58,7 @@ timestamp: 2026-07-23T00:10:00Z | `frontend/src/app/core/services/authenticated-event-source.service.ts` | Fetch-based authenticated SSE transport with frame parsing, abort support, and a separate `'unauthorized'` event for `401`/`403`. | | `frontend/src/app/core/services/bootstrap-event.service.ts` | Root-provided public SSE listener that subscribes to `/api/v1/event/stream`, filters `topic === 'general'` frames, and triggers `BootstrapService.refresh()` so the landing modal stays in sync with admin general-settings updates. | | `frontend/src/app/core/services/bootstrap-event.pure.ts` | Pure helpers for the public bootstrap SSE listener: `isBootstrapGeneralFrame` predicate, SSE line parser, and the `makeBootstrapEventSource` factory (fetch + `ReadableStream` opener with frame buffering and idempotent `close()`). | -| `frontend/src/app/core/services/event-status.store.ts` | Event state signal store, one-second countdown timer, and the optional `onUnauthorized` callback wiring. | +| `frontend/src/app/core/services/event-status.store.ts` | Event state signal store, one-second countdown timer, and the optional `onUnauthorized` callback wiring; exposes `start()`/`stop()`/`closeTransport()` for transport lifecycle and `subscribeReloadAtCountdownZero(handler)` (with `reloadAtCountdownZero(handler)` kept as a back-compat wrapper) so the reload handler is owned by its subscriber rather than the transport. | | `frontend/src/app/core/services/event-status.pure.ts` | Event payload types, transport interface (including `'unauthorized'` listener), pure countdown helpers, and the `LED_COLOR_BY_STATE` map used by the shell LED. | | `frontend/src/app/features/home/home.component.ts` | Authenticated shell container; starts user and event state services, subscribes to peer invalidation, and navigates to `/login` when the session is invalidated elsewhere. | | `frontend/src/app/features/shell/header/shell-header.component.ts` | Shell title, status LED (size/shape only — color comes from the pure map via `[style.background-color]`), countdown, and user menu. | @@ -72,7 +72,7 @@ timestamp: 2026-07-23T00:10:00Z | `tests/frontend/authenticated-event-source.spec.ts` | Tests SSE authorization, frame transport behavior, and the `401`/`403` unauthorized path. | | `tests/frontend/auth-session-events.spec.ts` | Pure tests for cross-tab invalidation message encoding, payload validation, and `storage`-event filtering. | | `tests/frontend/admin-challenges-import-autoclose.spec.ts` | Pure helpers (`summarizeImportResult`, `importErrorMessage`) and modal auto-close branch for the confirmed-import handler. | -| `frontend/src/app/features/challenges/challenges.page.ts` | `/challenges` smart page: gate logic (countdown/running/stopped/unconfigured), modal lifecycle, SSE wiring on `/api/v1/events`, and the countdown-zero reload. | +| `frontend/src/app/features/challenges/challenges.page.ts` | `/challenges` smart page: gate logic (countdown/running/stopped/unconfigured), modal lifecycle, SSE wiring on `/api/v1/events`, and the page-owned countdown-zero reload (handler registered in the constructor with the disposer wired into `DestroyRef`). | | `frontend/src/app/features/challenges/challenges.store.ts` | Signal store: board, event state, per-card solve listeners, SSE solve-frame mutation, submit response application, stop() lifecycle. | | `frontend/src/app/features/challenges/challenges.service.ts` | HTTP service for `/api/v1/challenges/{board,status,:id,:id/solves}` returning typed `ApiErrorEnvelope`s. | | `frontend/src/app/features/challenges/challenges.pure.ts` | Pure types and helpers (`BoardCard`, `SolverRow`, `SolveEventPayload`, `parseSolveEvent`, `mergeSolveEventIntoSolvers`, `messageForSolveError`, `formatDdHhMm`). | diff --git a/docs/guides/challenges-board.md b/docs/guides/challenges-board.md index c9d53af..81f7abc 100644 --- a/docs/guides/challenges-board.md +++ b/docs/guides/challenges-board.md @@ -66,8 +66,13 @@ renders `[data-testid="challenges-gate"]`: When `secondsToStart` or `secondsToEnd` reach zero, the `ChallengesPage` triggers a full `window.location.reload()` via -`EventStatusStore.reloadAtCountdownZero(...)` so the SPA re-evaluates -the board visibility from the server snapshot. +`EventStatusStore.subscribeReloadAtCountdownZero(...)` so the SPA +re-evaluates the board visibility from the server snapshot. The +subscription is page-owned: `ChallengesPage` registers the handler in +its constructor and wires the returned disposer into its `DestroyRef` +so the reload listener is cleared as soon as the page is destroyed, +independent of the store's `start()`/`stop()` transport lifecycle (see +[Event Window — Auto-reload at the transition boundary](/guides/event-window.md#auto-reload-at-the-transition-boundary)). ## Challenge modal diff --git a/docs/guides/event-window.md b/docs/guides/event-window.md index 5e60d44..718641e 100644 --- a/docs/guides/event-window.md +++ b/docs/guides/event-window.md @@ -3,7 +3,7 @@ type: guide title: Event Window description: How the event window state machine is computed and surfaced via REST + SSE. tags: [guide, event, countdown, sse, state-machine] -timestamp: 2026-07-22T09:03:53Z +timestamp: 2026-07-23T01:19:00Z --- # Configuration @@ -88,6 +88,24 @@ itself) close the SSE source and clear the tick interval, so leaving the shell (e.g. by logging out and landing on `/login`) tears the stream down cleanly. +# Auto-reload at the transition boundary + +`ChallengesPage` registers a one-shot handler with +`EventStatusStore.subscribeReloadAtCountdownZero(() => window.location.reload())` +in its constructor; the returned disposer is wired into its +`DestroyRef` so the subscription is cleared when the page is destroyed. +The subscription is **page-owned** and independent of the transport +lifecycle (`start()`/`stop()` only manage the SSE source + tick + +watcher; they never clear the reload handler). The store owns a 1 s +watcher (`checkZero`) that fires the handler exactly once when the +local-clock countdown reaches zero (`countdown → running`) or the +running countdown reaches zero (`running → stopped`). The latch is +re-armed on every state transition (countdown→running and +running→stopped), and a strict-equal follow-up SSE frame is collapsed +so a duplicate zero-countdown payload cannot disarm the latch. After +`stop()` (end-of-life teardown) the store is fully reset; a fresh +handler subscription re-installs the watcher. + # Push path 1. The admin cron / settings update writes to `setting.eventStartUtc` / diff --git a/frontend/src/app/core/services/event-status.store.ts b/frontend/src/app/core/services/event-status.store.ts index e2cc6c5..d8782d7 100644 --- a/frontend/src/app/core/services/event-status.store.ts +++ b/frontend/src/app/core/services/event-status.store.ts @@ -73,25 +73,38 @@ export class EventStatusStore { private zeroWatcher: ReturnType | null = null; private reloadOnZero: (() => void) | null = null; private reloadOnZeroFired = false; + private lastAppliedJson: string | null = null; constructor() { this.destroyRef.onDestroy(() => this.stop()); } applyServerStatus(payload: EventStatePayload): void { + const json = JSON.stringify(payload); + if (this.lastAppliedJson === json) { + // Same payload delivered again (e.g. SSE tick that duplicates the + // initial frame). Ignore it so the reload latch is not silently + // disarmed after the watcher has already fired. + return; + } + this.lastAppliedJson = json; + const isTransition = + this.state() !== payload.state || + this.eventStartUtc() !== payload.eventStartUtc || + this.eventEndUtc() !== payload.eventEndUtc; this.state.set(payload.state); this.serverNowUtc.set(payload.serverNowUtc); this.eventStartUtc.set(payload.eventStartUtc); this.eventEndUtc.set(payload.eventEndUtc); this.clientAnchorMs.set(Date.now()); // Reset the one-shot so a subsequent transition (e.g. countdown→running) can fire again. - if (this.reloadOnZeroFired) { + if (isTransition && this.reloadOnZeroFired) { this.reloadOnZeroFired = false; } } start(createSource: () => EventSourceLike, onUnauthorized?: () => void): void { - this.stop(); + this.closeTransport(); const src = createSource(); this.source = src; const handler = (ev: MessageEvent | Event) => { @@ -119,14 +132,48 @@ export class EventStatusStore { if (!this.intervalId) { this.intervalId = setInterval(() => this.tick.update((v) => v + 1), 1000); } - if (!this.zeroWatcher) { - this.zeroWatcher = setInterval(() => this.checkZero(), 1000); - } + this.ensureWatcher(); } reloadAtCountdownZero(handler: () => void): void { + this.subscribeReloadAtCountdownZero(handler); + } + + subscribeReloadAtCountdownZero(handler: () => void): () => void { this.reloadOnZero = handler; this.reloadOnZeroFired = false; + this.ensureWatcher(); + return () => { + if (this.reloadOnZero === handler) { + this.reloadOnZero = null; + this.reloadOnZeroFired = false; + } + }; + } + + private closeTransport(): void { + if (this.source) { + try { + this.source.close(); + } catch { + // ignore + } + this.source = null; + } + if (this.intervalId) { + clearInterval(this.intervalId); + this.intervalId = null; + } + if (this.zeroWatcher) { + clearInterval(this.zeroWatcher); + this.zeroWatcher = null; + } + } + + private ensureWatcher(): void { + if (!this.zeroWatcher) { + this.zeroWatcher = setInterval(() => this.checkZero(), 1000); + } } private checkZero(): void { @@ -137,12 +184,24 @@ export class EventStatusStore { if (s !== null && s <= 0) { this.reloadOnZeroFired = true; try { this.reloadOnZero(); } catch { /* ignore */ } + return; + } + const startMs = new Date(this.eventStartUtc() ?? '').getTime(); + if (Number.isFinite(startMs) && startMs <= Date.now()) { + this.reloadOnZeroFired = true; + try { this.reloadOnZero(); } catch { /* ignore */ } } } else if (state === 'running') { const s = this.secondsToEnd(); if (s !== null && s <= 0) { this.reloadOnZeroFired = true; try { this.reloadOnZero(); } catch { /* ignore */ } + return; + } + const endMs = new Date(this.eventEndUtc() ?? '').getTime(); + if (Number.isFinite(endMs) && endMs <= Date.now()) { + this.reloadOnZeroFired = true; + try { this.reloadOnZero(); } catch { /* ignore */ } } } } @@ -164,5 +223,8 @@ export class EventStatusStore { clearInterval(this.zeroWatcher); this.zeroWatcher = null; } + this.reloadOnZero = null; + this.reloadOnZeroFired = false; + this.lastAppliedJson = null; } } diff --git a/frontend/src/app/features/challenges/challenges.page.ts b/frontend/src/app/features/challenges/challenges.page.ts index 3b214fd..a2efece 100644 --- a/frontend/src/app/features/challenges/challenges.page.ts +++ b/frontend/src/app/features/challenges/challenges.page.ts @@ -127,6 +127,10 @@ export class ChallengesPage implements OnInit, OnDestroy { constructor() { this.destroyRef.onDestroy(() => this.store.stop()); + const unsubscribeReload = this.eventStatus.subscribeReloadAtCountdownZero(() => { + if (typeof window !== 'undefined') window.location.reload(); + }); + this.destroyRef.onDestroy(() => unsubscribeReload()); } ngOnInit(): void { @@ -147,9 +151,6 @@ export class ChallengesPage implements OnInit, OnDestroy { () => this.authenticatedEventSource.open('/api/v1/events'), ); } - this.eventStatus.reloadAtCountdownZero(() => { - if (typeof window !== 'undefined') window.location.reload(); - }); } ngOnDestroy(): void { diff --git a/tests/frontend/challenges-page.spec.ts b/tests/frontend/challenges-page.spec.ts index 34bb5a0..5cdc259 100644 --- a/tests/frontend/challenges-page.spec.ts +++ b/tests/frontend/challenges-page.spec.ts @@ -183,6 +183,26 @@ describe('ChallengesPage (gate + countdown + auto-reload wiring)', () => { expect(handler).toHaveBeenCalledTimes(2); }); + it('repeating the same zero-countdown SSE frame does not disarm the auto-reload latch (Job 905)', () => { + const handler = jest.fn(); + eventStore.reloadAtCountdownZero(handler); + + const frame = { + state: 'countdown' as const, + serverNowUtc: new Date().toISOString(), + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + eventEndUtc: new Date(Date.now() + 7200_000).toISOString(), + secondsToStart: 0, + secondsToEnd: null, + }; + eventStore.applyServerStatus(frame); + eventStore.applyServerStatus(frame); + eventStore.applyServerStatus(frame); + (eventStore as any).checkZero(); + + expect(handler).toHaveBeenCalledTimes(1); + }); + it('isRunning flips true only when state is running', async () => { apiSpy.getBoard.mockReturnValueOnce(of(makeBoard())); await challenges.load(); diff --git a/tests/frontend/event-status.store.watcher.spec.ts b/tests/frontend/event-status.store.watcher.spec.ts new file mode 100644 index 0000000..03b9176 --- /dev/null +++ b/tests/frontend/event-status.store.watcher.spec.ts @@ -0,0 +1,193 @@ +import { Injector, runInInjectionContext } from '@angular/core'; +import { EventStatusStore } from '../../frontend/src/app/core/services/event-status.store'; +import { EventStatePayload } from '../../frontend/src/app/core/services/event-status.pure'; + +describe('EventStatusStore auto-reload watcher (Job 905)', () => { + let store: EventStatusStore; + + function payload(over: Partial): EventStatePayload { + const start = new Date(Date.now() + 60_000).toISOString(); + const end = new Date(Date.now() + 7_200_000).toISOString(); + return { + state: 'countdown', + serverNowUtc: new Date().toISOString(), + eventStartUtc: start, + eventEndUtc: end, + secondsToStart: 60, + secondsToEnd: null, + ...over, + }; + } + + beforeEach(() => { + jest.useFakeTimers(); + const injector = Injector.create({ providers: [] }); + store = runInInjectionContext(injector, () => new EventStatusStore()); + }); + + afterEach(() => { + if (store) store.stop(); + jest.useRealTimers(); + }); + + it('fires the reload handler exactly once when secondsToStart crosses zero via the real setInterval path', () => { + const handler = jest.fn(); + store.reloadAtCountdownZero(handler); + + store.applyServerStatus( + payload({ state: 'countdown', secondsToStart: 2, eventStartUtc: new Date(Date.now() + 2000).toISOString() }), + ); + + jest.advanceTimersByTime(3000); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('re-arms the latch on a state transition so a later end-of-event zero reloads again', () => { + const handler = jest.fn(); + store.reloadAtCountdownZero(handler); + + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + expect(handler).toHaveBeenCalledTimes(1); + + store.applyServerStatus( + payload({ + state: 'running', + secondsToStart: null, + secondsToEnd: 9001, + eventStartUtc: new Date(Date.now() - 60_000).toISOString(), + eventEndUtc: new Date(Date.now() + 9001_000).toISOString(), + }), + ); + jest.advanceTimersByTime(1000); + expect(handler).toHaveBeenCalledTimes(1); + + store.applyServerStatus( + payload({ + state: 'running', + secondsToStart: null, + secondsToEnd: 0, + eventStartUtc: new Date(Date.now() - 60_000).toISOString(), + eventEndUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + + expect(handler).toHaveBeenCalledTimes(2); + }); + + it('registering a new handler replaces the previous one without double-firing', () => { + const first = jest.fn(); + const second = jest.fn(); + store.reloadAtCountdownZero(first); + store.reloadAtCountdownZero(second); + + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + }); + + it('stop() resets the latch so a fresh handler can fire on the next zero', () => { + const first = jest.fn(); + store.reloadAtCountdownZero(first); + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + expect(first).toHaveBeenCalledTimes(1); + + store.stop(); + + const second = jest.fn(); + store.reloadAtCountdownZero(second); + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + + expect(second).toHaveBeenCalledTimes(1); + }); + + it('does not disarm the latch when the same zero-countdown payload is delivered repeatedly', () => { + const handler = jest.fn(); + store.reloadAtCountdownZero(handler); + + const frame = payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }); + + store.applyServerStatus(frame); + store.applyServerStatus(frame); + store.applyServerStatus(frame); + jest.advanceTimersByTime(2000); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('subscribeReloadAtCountdownZero unregister prevents the handler from firing', () => { + const handler = jest.fn(); + const unsubscribe = store.subscribeReloadAtCountdownZero(handler); + + unsubscribe(); + + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('start() preserves a previously registered reload handler (Job 905 handler ownership)', () => { + const handler = jest.fn(); + store.subscribeReloadAtCountdownZero(handler); + + const fakeSource = { + addEventListener: jest.fn(), + close: jest.fn(), + }; + store.start(() => fakeSource as any); + + expect((store as any).reloadOnZero).toBe(handler); + + store.applyServerStatus( + payload({ + state: 'countdown', + secondsToStart: 0, + eventStartUtc: new Date(Date.now() - 1000).toISOString(), + }), + ); + jest.advanceTimersByTime(2000); + + expect(handler).toHaveBeenCalledTimes(1); + }); +}); \ No newline at end of file