From d85e7a519eb94d780706143c160e3b688da9ace8 Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Wed, 22 Jul 2026 09:04:43 +0000 Subject: [PATCH] feat: Authenticated Shell, Quick Tabs and Change Password 1.02 --- .kilo/plans/876.md | 55 ---------- .kilo/plans/878.md | 221 ++++++++++++++++++++++++++++++++++++++++ frontend/src/styles.css | 7 +- 3 files changed, 227 insertions(+), 56 deletions(-) delete mode 100644 .kilo/plans/876.md create mode 100644 .kilo/plans/878.md diff --git a/.kilo/plans/876.md b/.kilo/plans/876.md deleted file mode 100644 index b996021..0000000 --- a/.kilo/plans/876.md +++ /dev/null @@ -1,55 +0,0 @@ -# Implementation Plan: Authenticated Shell, Quick Tabs and Change Password 1.01 - -## 1. Architectural Reconnaissance -- **Status:** The requested shell, quick tabs, change-password flow, and authenticated SSE endpoint are already present, but Job 876 is **not** fully implemented because the authenticated browser SSE request is rejected with 401 and the LED remains `led-unconfigured`. Existing implementation is concentrated in `frontend/src/app/features/home/home.component.ts:127-135`, `frontend/src/app/core/services/event-status.store.ts:51-76`, `frontend/src/app/features/shell/header/shell-header.component.ts:25-35`, and `backend/src/modules/system/system.controller.ts:77-113`. -- **Codebase style & conventions:** TypeScript monorepo with NestJS 10 backend and Angular 17 standalone frontend. Angular uses OnPush components, signal inputs/outputs, signal-backed services, and a smart `HomeComponent` container with dumb shell children. NestJS uses a global JWT guard, `@Public()` for public routes, typed controller methods, RxJS Observables for SSE, and TypeORM over SQLite. -- **Data Layer:** better-sqlite3 through TypeORM, with event window values stored in `setting` rows (`eventStartUtc` and `eventEndUtc`). No schema change is indicated. `EventStatusService.getState()` is the authoritative four-state event state machine. -- **Authentication / transport:** REST requests receive `Authorization: Bearer ` from `authInterceptor` (`frontend/src/app/core/interceptors/auth.interceptor.ts:5-11`). The browser-native `EventSource` created by `HomeComponent` only sends cookies via `{ withCredentials: true }`; it does not send the interceptor's bearer header. The backend's global `JwtAuthGuard` validates bearer JWTs, and the plural `/api/v1/events/status` endpoint is intentionally authenticated SSE. -- **Test Framework & Structure:** Jest 29 with ts-jest, split into `tests/backend` (Node/Supertest integration tests) and `tests/frontend` (jsdom/pure frontend tests); root `npm test` runs both projects via `tests/jest.config.js`. Existing SSE coverage verifies unauthenticated 401 and authenticated initial frames in `tests/backend/events-status-sse.spec.ts`; existing event-status frontend coverage is currently pure-helper focused in `tests/frontend/event-status.store.spec.ts`. -- **Required Tools & Dependencies:** No new dependency should be required. The implementation should use existing Angular, NestJS, RxJS, JWT, and Jest packages. No database migration or `/data` fixture is needed. `setup.sh` already installs dependencies and builds both workspaces; retain that behavior unless the chosen authentication transport requires an existing package already present in the lockfile. - -## 2. Impacted Files -- **To Modify:** - - `frontend/src/app/features/home/home.component.ts` — change the authenticated SSE connection strategy so the request carries the access token accepted by the backend, while preserving shell lifecycle cleanup. - - `frontend/src/app/core/services/event-status.store.ts` and/or `frontend/src/app/core/services/event-status.pure.ts` — only if needed to support the selected authenticated event-source abstraction or validate the incoming frame contract without moving state logic into the component. - - `frontend/src/app/core/interceptors/auth.interceptor.ts` — only if the final design introduces a reusable authenticated streaming transport through Angular HTTP; otherwise leave unchanged because native `EventSource` bypasses interceptors. - - `backend/src/modules/system/system.controller.ts` — only if the selected design intentionally changes the stream authentication contract (for example, a narrowly scoped query-token fallback); avoid weakening the existing JWT protection unless required and explicitly constrained. - - `tests/frontend/event-status.store.spec.ts` or a new focused file under `tests/frontend/` — add a minimal regression test proving the shell status store receives and applies an authenticated stream frame through the chosen abstraction. - - `tests/backend/events-status-sse.spec.ts` — retain the existing unauthenticated rejection and authenticated initial-frame tests; extend only if the backend contract changes. - - `docs/api/system.md` and `docs/guides/event-window.md` — update the documented browser authentication mechanism if the implementation changes the current native `EventSource` contract. -- **To Create:** - - Prefer no new production file unless a small dedicated authenticated SSE client service is required by the selected approach. - - If extraction is needed, create `frontend/src/app/core/services/authenticated-event-source.service.ts` and place its focused unit test under `tests/frontend/`, not beside source code. - -## 3. Proposed Changes -Explain the exact implementation logic step-by-step: - -1. **Database / Schema Migration:** - - No migration. Continue reading `eventStartUtc` and `eventEndUtc` from the existing `setting` table through `EventStatusService`. - - Confirm the fix preserves the current public endpoints (`GET /api/v1/event/status` and `GET /api/v1/settings/event`) and the authenticated SSE payload shape (`state`, timestamps, and countdown fields). - -2. **Backend Logic & APIs:** - - Treat the observed 401 as an authentication-transport mismatch, not an event-state calculation problem: `HomeComponent` uses native `EventSource`, while the backend JWT strategy extracts only an Authorization bearer token. - - Prefer a frontend-only transport fix that keeps `/api/v1/events/status` JWT-protected. The cleanest option is to replace the native `EventSource` connection with an existing-API-compatible authenticated streaming client that can issue a GET carrying the current bearer token and parse SSE `message` frames, then expose the same `EventSourceLike` lifecycle (`addEventListener`/`close`) consumed by `EventStatusStore.start()`. - - Keep authentication centralized: obtain the current access token from `AuthService`, send it as `Authorization: Bearer `, include credentials where refresh-cookie behavior requires it, and ensure the stream is closed when the shell is destroyed or the session is cleared. - - Do not make `/api/v1/events/status` public and do not accept unrestricted unauthenticated query tokens. If a query-token fallback is considered unavoidable because of browser-native EventSource limitations, document and constrain it, avoid logging the token, and add explicit backend tests for valid/invalid/expired token behavior; the default plan is to avoid this security regression. - - Preserve the existing RxJS SSE implementation: it should continue to emit the initial state immediately, tick every 60 seconds, merge hub updates, and suppress identical payloads. No changes to event-state computation are needed. - -3. **Frontend UI Integration:** - - In `HomeComponent.ngOnInit()`, start the stream only after the authenticated shell is mounted and the current access token is available; pass the authenticated source factory into `EventStatusStore.start()`. - - Keep `EventStatusStore` responsible for parsing frames, applying `EventStatePayload`, maintaining the server-clock anchor, and ticking local countdown signals. Do not duplicate status logic in `HomeComponent` or `ShellHeaderComponent`. - - Keep `ShellHeaderComponent`'s state-class bindings and ARIA label intact. Once the first authenticated frame arrives, `eventStatus.state()` must become `running` for the reported active window, resulting in `led-running` rather than the initial `led-unconfigured` class. - - Preserve quick-tab routing and the existing change-password flow; repository inspection shows those features already exist and are unrelated to the 401 root cause. Avoid reworking them. - - Ensure stream errors do not leave an uncancelled reader, timer, or subscription. Reconnect behavior should be limited to the existing lifecycle unless a focused retry is necessary; do not create an aggressive reconnect loop that can amplify authentication failures. - - Update the relevant API/event-window documentation to state how the authenticated browser stream transmits JWT credentials after the fix. - -## 4. Test Strategy -- **Target Unit Test File:** - - Add the smallest frontend regression coverage under `tests/frontend/`, preferably in `tests/frontend/event-status.store.spec.ts` if the abstraction remains store-compatible, or in a new `tests/frontend/authenticated-event-source.spec.ts` if a dedicated client is extracted. - - Arrange a fake authenticated stream boundary and a valid `EventStatePayload` with a running event window; act by starting the store/client; assert that the payload is applied and the resulting state is `running`. Also assert the request boundary receives the bearer token if the new client owns request creation. - - Keep the existing backend tests in `tests/backend/events-status-sse.spec.ts`: the 401 test is essential evidence that the endpoint remains protected, and the authenticated initial-frame test verifies the server contract. If no backend route contract changes, do not add broad backend scenarios. -- **Mocking Strategy:** - - Mock only external boundaries: the browser stream/request implementation, `AuthService` token lookup where needed, timers/Date only if required for deterministic countdown behavior, and network responses. Do not mock `EventStatusService` in the existing backend integration test; it is fast internal logic and the current test already uses the real module and isolated in-memory database. - - Use the repository's Jest configuration and dedicated `tests/` folders. Tests must run with the single root command `npm test`, must not require a browser or visual assertion, and must not introduce persistent fixtures. If any fixture is needed, use `/data` and generate it when absent, but this job should need none. - - Follow Arrange-Act-Assert and keep assertions focused on the authentication header/stream frame and LED-driving state contract. Avoid visual tests, large mock environments, or extensive edge-case matrices. -- **Verification:** After implementation, run the repository's available root test command plus the workspace build/type validation commands used by the project. There is no declared lint or typecheck npm script in `package.json`; implementers should at minimum run `npm test` and `npm run build`, and use the workspace TypeScript/build commands if separate validation is required. diff --git a/.kilo/plans/878.md b/.kilo/plans/878.md new file mode 100644 index 0000000..7f3d8c8 --- /dev/null +++ b/.kilo/plans/878.md @@ -0,0 +1,221 @@ +# Implementation Plan: Authenticated Shell — LED Color Rendering (Job 878, 1.02) + +## 0. Already-Implemented Check + +After investigating the repository: + +- `frontend/src/app/features/shell/header/shell-header.component.ts` already + renders `` + with the correct `aria-label`. +- The state derivation (`running` / `countdown` / `stopped` / `unconfigured`) + is already correct (verified by the Job description: `running` between + Start and End is being assigned). +- `frontend/src/styles.css` defines the color tokens + `--color-success`, `--color-warning`, `--color-danger`, but contains **no** + rules for `.led`, `.led-running`, `.led-countdown`, `.led-stopped`, or + `.led-unconfigured`. + +The bug is therefore isolated to missing CSS. The fix is purely a stylesheet +addition. No TypeScript / Angular code changes are required; no backend, no +state logic, and no template changes are required. + +This means most sections of this plan are intentionally short — there is +nothing to migrate, no APIs to add, and no component logic to author. + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** TypeScript, Angular 17+ standalone + components (`ShellHeaderComponent` is `standalone: true`, + `ChangeDetectionStrategy.OnPush`). Global styles live in + `frontend/src/styles.css` and are the single source of design tokens + (`--color-*`, `--radius-*`, `--font-family`). All Angular component + templates and styles are inlined per-component; nothing for the LED is + defined at component scope. +- **Data Layer:** Not applicable — this Job is a pure CSS fix. The event state + itself already flows correctly via `EventStatusStore` → + `HomeComponent` → `ShellHeaderComponent.eventState` input. +- **Test Framework & Structure:** Jest via `tests/jest.config.js` with two + projects (`backend`, `frontend`). Frontend tests use `jsdom` + `ts-jest` and + `tests/frontend/jest.setup.ts` (provides `matchMedia` shim). Frontend specs + live under `tests/frontend/*.spec.ts` and import directly from + `frontend/src/...`. All tests run from the repo root with a single command: + `npm test`. +- **Required Tools & Dependencies:** None new. Existing stack (Node, Angular + CLI / `npm --workspace frontend run build`, Jest 29, jsdom 29) is + sufficient. + +## 2. Impacted Files + +- **To Modify:** + - `frontend/src/styles.css` — add the four `.led-{state}` rules plus the + base `.led` rule, using the already-defined color tokens. One-line + reason: the LED element currently has no dimensions and no color, so it + renders as invisible content; the bug description confirms this + exactly. +- **To Create:** + - `tests/frontend/led-class-mapping.spec.ts` — minimal regression test + that asserts `ShellHeaderComponent.ledClass()` produces the expected + `led led-` string for each `EventState` value. This verifies the + class wiring that the new CSS rules depend on (small, pure, runnable + without a UI). + +## 3. Proposed Changes + +### 3.1 Stylesheet (`frontend/src/styles.css`) + +Append the following rules to the bottom of `frontend/src/styles.css`, +preserving the existing two-space indentation used in the file: + +```css +.led { + display: inline-block; + width: 10px; + height: 10px; + border-radius: 50%; + border: 0; + background-color: transparent; + vertical-align: middle; +} +.led-running { background-color: var(--color-success); } +.led-countdown { background-color: var(--color-warning); } +.led-stopped { background-color: var(--color-danger); } +.led-unconfigured { background-color: var(--color-secondary); } +``` + +Design rationale (each decision is required to satisfy the Job): + +- `display: inline-block` + `width/height: 10px` + `border-radius: 50%` are + the minimum declarations needed to paint a visible circular dot and to + override the browser's default `display: inline` on the `` (which + the bug report explicitly calls out as `width: auto; height: auto; + background-color: rgba(0,0,0,0)`). +- The four state classes map directly to the four token colors already + declared in `:root`, which is exactly the mapping the Job specifies + (Green / Yellow / Red / secondary-gray for unconfigured, since + `--color-secondary` was the only remaining neutral token available). +- No `::before` / `::after` content is used, so no `content` property is + needed (the bug report ruled these out as a fix path). +- No changes to the template or component logic — the `[class.led-running]` + etc. bindings in `shell-header.component.ts` already drive the right + state class. + +### 3.2 Regression test (`tests/frontend/led-class-mapping.spec.ts`) + +This test is purely defensive: it locks in the contract that +`ShellHeaderComponent.ledClass` produces the class string the CSS rules are +keyed on, so a future template refactor cannot silently break the visual +fix. It uses the existing `jsdom` Jest project and the same import style as +`tests/frontend/shell-active-section.spec.ts`. + +```ts +import { ShellHeaderComponent } from + '../../frontend/src/app/features/shell/header/shell-header.component'; +import { EventState } from + '../../frontend/src/app/core/services/event-status.store'; + +describe('ShellHeaderComponent LED class mapping', () => { + function build() { + const c = new ShellHeaderComponent(); + return c; + } + + it.each([ + ['running', 'led led-running'], + ['countdown', 'led led-countdown'], + ['stopped', 'led led-stopped'], + ['unconfigured', 'led led-unconfigured'], + ] as [EventState, string][])( + 'maps event state %s to class string %s', + (state, expected) => { + const c = build(); + c.eventState.set(state as any); // input signal set is a noop — handled below + }, + ); +}); +``` + +Because `ShellHeaderComponent` exposes `eventState` as an Angular **input +signal** (`input('unconfigured')`), and test-driving Angular +signals without `TestBed` is fragile, the cleanest minimal test — and the +one most consistent with the existing convention used by +`shell-active-section.spec.ts` (which tests a pure helper) — is to mirror +that pattern. We will instead add **one lightweight pure helper** export +from `shell-header.component.ts`: + +```ts +// pure helper, exported for tests +export function ledClassForState(state: EventState): string { + return `led led-${state}`; +} +``` + +And have `ledClass` delegate to it: + +```ts +readonly ledClass = computed(() => ledClassForState(this.eventState())); +``` + +This keeps the test pure-function-style (matches the +`deriveActiveSectionFromUrl` precedent in `home.shell.ts`) and avoids any +`TestBed` / `ChangeDetection` setup. The test then becomes: + +```ts +import { ledClassForState } from + '../../frontend/src/app/features/shell/header/shell-header.component'; + +describe('ledClassForState', () => { + it('returns the base + state class for each EventState', () => { + expect(ledClassForState('running')) + .toBe('led led-running'); + expect(ledClassForState('countdown')) + .toBe('led led-countdown'); + expect(ledClassForState('stopped')) + .toBe('led led-stopped'); + expect(ledClassForState('unconfigured')) + .toBe('led led-unconfigured'); + }); +}); +``` + +A single `describe` / single `it` keeps the suite minimal and instantly +runnable, satisfying the "minimal, highly-focused tests" rule. + +If the implementer prefers to avoid touching `shell-header.component.ts` +entirely, the alternative is to skip this test. The plan therefore lists +this test as **optional / lowest priority**, and the Plan Implementer may +omit it without affecting the bug fix. The bug fix does **not** require +any code change to `shell-header.component.ts`. + +## 4. Test Strategy + +- **Target Unit Test File:** `tests/frontend/led-class-mapping.spec.ts` + (optional; see §3.2). Run via `npm test` from the repo root, which the + project's package.json already wires to + `jest --config tests/jest.config.js`. +- **Mocking Strategy:** None required. The test, if added, exercises a + pure exported function with no I/O, no Angular DI, and no DOM mutation, + so no mocks, fakes, or `TestBed` configuration are needed. The existing + `tests/frontend/jest.setup.ts` already shims `matchMedia` for any + incidental DOM usage. +- **What we deliberately do NOT add:** + - No visual regression test (the Job explicitly forbids tests that + require visual confirmation). + - No `TestBed` rendering test for `ShellHeaderComponent` (would add + infrastructure cost for no functional value, since the existing + template + class bindings are unchanged). + - No backend or integration test — the bug is purely CSS. + +## 5. Acceptance Criteria + +The Job is considered fixed when, after rebuilding the Angular bundle: + +1. The `` element is rendered as a 10×10 + filled circle in the authenticated shell header. +2. Its background color matches the live event state: + - `countdown` (now → startUtc) → `var(--color-warning)` (#f59e0b / yellow) + - `running` (startUtc → endUtc) → `var(--color-success)` (#10b981 / green) + - `stopped` (after endUtc) → `var(--color-danger)` (#ef4444 / red) + - `unconfigured` (no Start/End) → `var(--color-secondary)` neutral +3. No new HTTP traffic, no new backend code, no new Angular component + file, and no change to the shell template. +4. `npm test` continues to pass. diff --git a/frontend/src/styles.css b/frontend/src/styles.css index 020c3b7..d656dc2 100644 --- a/frontend/src/styles.css +++ b/frontend/src/styles.css @@ -19,4 +19,9 @@ button { font: inherit; padding: 8px 16px; border: 0; border-radius: var(--radiu button[disabled] { opacity: .5; cursor: not-allowed; } input { font: inherit; padding: 8px 12px; border: 1px solid var(--color-secondary); border-radius: var(--radius-sm); width: 100%; } .container { max-width: 960px; margin: 0 auto; padding: 24px; } -.card { padding: 24px; border-radius: var(--radius-lg); box-shadow: 0 1px 4px rgba(0,0,0,.1); } \ No newline at end of file +.card { padding: 24px; border-radius: var(--radius-lg); box-shadow: 0 1px 4px rgba(0,0,0,.1); } +.led { display: inline-block; width: 10px; height: 10px; border-radius: 50%; border: 0; background-color: transparent; vertical-align: middle; } +.led-running { background-color: var(--color-success); } +.led-countdown { background-color: var(--color-warning); } +.led-stopped { background-color: var(--color-danger); } +.led-unconfigured { background-color: var(--color-secondary); } \ No newline at end of file