From 8fbc68f99313176ddd185ff1773f7225a1e6991a Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 11 Jun 2026 18:28:58 -0400 Subject: [PATCH 01/24] docs(13): capture phase context --- .../13-real-lint-gate-eslint/13-CONTEXT.md | 114 ++++++++++++++++++ .../13-DISCUSSION-LOG.md | 91 ++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 .planning/phases/13-real-lint-gate-eslint/13-CONTEXT.md create mode 100644 .planning/phases/13-real-lint-gate-eslint/13-DISCUSSION-LOG.md diff --git a/.planning/phases/13-real-lint-gate-eslint/13-CONTEXT.md b/.planning/phases/13-real-lint-gate-eslint/13-CONTEXT.md new file mode 100644 index 0000000..ca13718 --- /dev/null +++ b/.planning/phases/13-real-lint-gate-eslint/13-CONTEXT.md @@ -0,0 +1,114 @@ +# Phase 13: Real Lint Gate (ESLint) - Context + +**Gathered:** 2026-06-11 +**Status:** Ready for planning + + +## Phase Boundary + +Stand up a real ESLint flat config across both workspaces (`apps/api` — NodeNext ESM Node/TS; `apps/pwa` — React 19 + Bundler ESM), plus package-level `lint` scripts, so the existing CI step `pnpm lint` actually fails on violations instead of exiting 0 as a no-op. + +**Scope expanded during discussion (flag for planning):** This phase now also adds **Prettier** with a standalone `prettier --check` CI format gate. The original ROADMAP Phase 13 one-liner scopes to "ESLint only" — planning MUST update the ROADMAP entry and success criteria to include the format gate. Treated as an in-domain expansion (still a CI quality gate; CLAUDE.md's stack table lists "ESLint + Prettier"), not a separate phase. + +**In scope:** +- ESLint flat config (`eslint.config.js`) — typescript-eslint + React + react-hooks plugins +- Package-level `lint` scripts in `apps/api` and `apps/pwa` (so root `pnpm -r --if-present lint` runs a real linter) +- Prettier config + root `format` / `format:check` scripts +- A new `format:check` CI step in the `fast-checks` job +- Fixing all first-run violations so `pnpm lint` AND `pnpm format:check` are green across both apps + +**Out of scope:** +- CI lint-step plumbing — `.gitea/workflows/ci.yml` already runs `pnpm lint`; the slot auto-activates once package `lint` scripts exist. Only the NEW `format:check` step is added. +- Desktop E2E coverage (Phase 14), any non-lint CI changes. + + + +## Implementation Decisions + +### Ruleset & strictness +- **D-13-01:** Use typescript-eslint **`recommendedTypeChecked`** (type-aware), not the non-type-aware `recommended`. Rationale: this is an async-heavy backend (outbox drain, push dispatch, CalDAV/reminder schedulers) — type-aware rules catch floating promises, `no-misused-promises`, and unsafe `any` that syntactic linting misses. Enable via `projectService: true` (resolves all tsconfigs automatically). +- **D-13-02:** Add React + react-hooks plugins for `apps/pwa` (per ROADMAP goal). `apps/api` is Node/TS only (no React config). +- **D-13-03:** Do NOT adopt `strict`/`strictTypeChecked` presets — too much churn on the existing 91-file codebase; CLAUDE.md warns against bikeshedding. + +### Gate threshold +- **D-13-04:** Run with **`--max-warnings 0`** — any warning fails CI. Every rule must be a real decision: either error-worthy or off. No non-blocking warnings (they rot into ignored noise). + +### First-run violation strategy +- **D-13-05:** **Fix all violations now.** The phase is not done until `pnpm lint` and `pnpm format:check` are green across both apps. Real bugs (floating promises, misused promises) get genuinely fixed. +- **D-13-06 (HARD CONSTRAINT — for executors):** Fixes must **address** the violation, not mask it. No blanket `eslint-disable` and no `void promise` to silence a floating-promise that should actually be `await`ed. Any suppression (`eslint-disable-next-line`) requires a justifying inline comment explaining why the rule is wrong *here*. A type-aware lint finding is a candidate bug — review before suppressing. + +### Prettier +- **D-13-07:** Add **Prettier + standalone `prettier --check`** as its own CI step (separate from lint), AND add **`eslint-config-prettier`** to the flat config to disable ESLint's formatting rules. Clean separation: ESLint finds bugs, Prettier owns formatting, no double-reporting. (Rejected: `eslint-plugin-prettier` — slower, noisier, discouraged by Prettier docs.) +- **D-13-08:** All files get reformatted in this phase — accepted as one large mechanical diff. Planning should consider isolating the reformat commit from logic fixes for reviewability. + +### File coverage +- **D-13-09:** Lint **all TS/TSX**: app `src/`, vitest tests, Playwright e2e specs (`apps/pwa` uses `tsconfig.e2e.json`), and config files (`vite.config`, `drizzle.config`, `playwright.config`). +- **D-13-10:** Config files / non-project files that `projectService` can't type-check need a dedicated override block (non-type-checked rules, or `disableTypeChecked` for those globs) so type-aware linting doesn't error on them. + +### Claude's Discretion +- Flat-config file layout (single root `eslint.config.js` vs per-app configs) — planner/researcher decides; root config with per-package overrides is the common pattern for a small 2-app pnpm workspace. +- Exact Prettier options (`.prettierrc`) — standard defaults; no bikeshedding. +- CI step ordering within `fast-checks` (lint → format:check → typecheck → tests). + + + + +## Canonical References + +**Downstream agents MUST read these before planning or implementing.** + +### CI integration (the slot this phase fills) +- `.gitea/workflows/ci.yml` — the `fast-checks` job runs `pnpm lint` (currently a no-op; see the inline comment at the Lint step). This phase makes it real and adds a `format:check` step. No other CI plumbing change. +- `package.json` (root) — `lint: pnpm -r --if-present lint`, `typecheck: pnpm -r typecheck`. Add root `format` / `format:check`. + +### Workspace / TS config (constrains the flat config) +- `apps/api/tsconfig.json` — NodeNext ESM, `target ES2023`, `strict`, excludes `tests`. `type: module`. +- `apps/pwa/tsconfig.json` — ESNext / Bundler resolution, `jsx: react-jsx`, `strict`, `noEmit`. `type: module`. +- `apps/pwa/tsconfig.e2e.json` — separate project for Playwright specs; must be in the lint projectService set for type-aware linting of e2e tests. +- `pnpm-workspace.yaml` — packages: `apps/*`. + +### Stack guidance +- `CLAUDE.md` (Development Tools table) — "ESLint + Prettier | Standard config; no bikeshedding needed." TypeScript 5.x `strict: true`. + +No external ADRs/specs specific to linting — requirements fully captured in decisions above. + + + + +## Existing Code Insights + +### Reusable Assets +- None to reuse — greenfield lint config. No ESLint or Prettier anywhere in the repo today (confirmed: no eslint dep in any `package.json`, no config files). + +### Established Patterns +- Both apps are `type: module` ESM → flat config file must be `eslint.config.js` (ESM) or `.mjs`. +- API and PWA each already have a `typecheck` script (`tsc --noEmit`) wired into the root `pnpm -r typecheck` — mirror that wiring style for the new package-level `lint` scripts. +- Type-aware linting needs every linted file resolvable by a tsconfig project. PWA has TWO tsconfigs (`tsconfig.json` + `tsconfig.e2e.json`) — `projectService: true` handles multi-project resolution. + +### Integration Points +- `.gitea/workflows/ci.yml` `fast-checks` job — `pnpm lint` step already present (activates automatically); ADD a `pnpm format:check` step. +- File volume for the first-run fix pass: `apps/api/src` = 30 `.ts` files; `apps/pwa/src` = 61 `.ts/.tsx` files, plus tests, e2e specs, and config files. + + + + +## Specific Ideas + +- Type-aware ruleset is specifically motivated by the async surfaces in this codebase: `apps/api/src/broker/` (outbox worker, poller, reminder scheduler — all recently bug-prone per STATE.md: node-cron→setInterval, reminder catch-up). Floating-promise / misused-promise detection here has concrete bug-catching value, not just style. + + + + +## Deferred Ideas + +None — discussion stayed within phase scope (Prettier was folded IN as an accepted scope expansion, not deferred). + +### Reviewed Todos (not folded) +- **Gitea CI — full regression on PR to main + build/publish Docker image** (`2026-06-10-gitea-ci-regression-and-docker-publish.md`, score 0.6) — reviewed but NOT folded: this is the Phase 8 CI work, already complete and merged to `main`. Matched only on shared keywords (CI/apps/api). Not in Phase 13 scope. + + + +--- + +*Phase: 13-real-lint-gate-eslint* +*Context gathered: 2026-06-11* diff --git a/.planning/phases/13-real-lint-gate-eslint/13-DISCUSSION-LOG.md b/.planning/phases/13-real-lint-gate-eslint/13-DISCUSSION-LOG.md new file mode 100644 index 0000000..6757997 --- /dev/null +++ b/.planning/phases/13-real-lint-gate-eslint/13-DISCUSSION-LOG.md @@ -0,0 +1,91 @@ +# Phase 13: Real Lint Gate (ESLint) - Discussion Log + +> **Audit trail only.** Do not use as input to planning, research, or execution agents. +> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered. + +**Date:** 2026-06-11 +**Phase:** 13-real-lint-gate-eslint +**Areas discussed:** Rule strictness, Existing-violation strategy, Prettier scope, Lint file coverage + +--- + +## Rule Strictness — ruleset + +| Option | Description | Selected | +|--------|-------------|----------| +| `recommended` (non-type-aware) | Syntactic only, fast, no tsconfig wiring; misses async/promise bugs | | +| `recommendedTypeChecked` | Type-aware; catches floating/misused promises + unsafe any; needs projectService, slower, more first-run violations | ✓ | +| `strict`/`strictTypeChecked` | Maximal rules; heavy churn + bikeshed risk | | + +**User's choice:** `recommendedTypeChecked` +**Notes:** Motivated by async-heavy backend (outbox/push/reminder schedulers). projectService: true. + +## Rule Strictness — gate threshold + +| Option | Description | Selected | +|--------|-------------|----------| +| `--max-warnings 0` | Any warning fails CI; every rule is error-or-off | ✓ | +| Errors only | Warnings surface but don't fail; softer rollout, accumulation risk | | +| You decide | Pick based on first-run count | | + +**User's choice:** `--max-warnings 0` + +--- + +## Existing-Violation Strategy + +| Option | Description | Selected | +|--------|-------------|----------| +| Fix all violations now | Phase green across both apps; real bugs fixed; larger phase | ✓ | +| Minimal green ruleset, ratchet later | Enable only passing rules; small phase, weaker gate, deferred work | | +| Baseline file (snapshot + ratchet) | Snapshot current violations as accepted; new-only fails; extra tooling | | + +**User's choice:** Fix all violations now +**Notes:** Hard constraint added (D-13-06): fixes must address the violation, not mask it — no blanket eslint-disable, no `void` to silence floating promises; suppressions need justifying comments. + +--- + +## Prettier Scope + +| Option | Description | Selected | +|--------|-------------|----------| +| `eslint-config-prettier` only | Disable conflicting format rules, no Prettier itself; phase stays ESLint-only | | +| Add Prettier + format gate too | Full lint+format; scope creep beyond goal; reformats all files now | ✓ | +| Defer Prettier entirely | ESLint only, no config-prettier; future conflict risk | | + +**User's choice:** Add Prettier + format gate too +**Notes:** Expands ROADMAP Phase 13 goal ("ESLint only") — flagged for planning to update ROADMAP one-liner + success criteria. Follow-up decided wiring: standalone `prettier --check` + eslint-config-prettier (rejected eslint-plugin-prettier). + +### Prettier wiring (follow-up) + +| Option | Description | Selected | +|--------|-------------|----------| +| Standalone + config-prettier | Separate `prettier --check` CI step + eslint-config-prettier off-switch; clean separation | ✓ | +| eslint-plugin-prettier | Prettier as an ESLint rule; one gate, slower/noisier, discouraged | | + +**User's choice:** Standalone + eslint-config-prettier + +--- + +## Lint File Coverage + +| Option | Description | Selected | +|--------|-------------|----------| +| All TS/TSX: src + tests + e2e + configs | Most thorough; needs projectService + config-file override; more first-run fixes | ✓ | +| src/ + tests, skip configs | Simpler, configs unchecked | | +| src/ only | Smallest scope; tests + e2e harness + configs unlinted | | + +**User's choice:** All TS/TSX (src + tests + e2e + configs) +**Notes:** Config files need a non-type-checked override block (D-13-10). + +--- + +## Claude's Discretion + +- Flat-config file layout (single root vs per-app) +- Exact `.prettierrc` options (standard defaults) +- CI step ordering within `fast-checks` + +## Deferred Ideas + +None — Prettier was folded into scope, not deferred. Reviewed-but-not-folded: the Phase 8 Gitea-CI todo (already complete, matched on shared keywords only). -- 2.54.0 From 2b6b0da93909b9a14c87b19b76457ca0624e9b87 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 11 Jun 2026 18:29:04 -0400 Subject: [PATCH 02/24] docs(state): record phase 13 context session --- .planning/STATE.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.planning/STATE.md b/.planning/STATE.md index e8cd6b4..d85d61d 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,12 +2,12 @@ gsd_state_version: 1.0 milestone: v1.1 milestone_name: Operability & Polish -status: phase-complete -stopped_at: Phase 08 complete — all 4 plans executed, CI-01 + CI-02 delivered, publish job verified green (run #14) -last_updated: "2026-06-11T22:00:00.000Z" -last_activity: "2026-06-11 -- 08-04 complete; publish job green (run #14, merge commit 98acff8): both image tags pushed (familysync-api:latest + :v1.1-98acff8), PAT masked, --password-stdin confirmed. REGISTRY_PAT naming fix (73eecf7). Phase 8 (Gitea CI) complete — all 6 ROADMAP criteria met." +status: completed +stopped_at: Phase 13 context gathered +last_updated: "2026-06-11T22:29:04.078Z" +last_activity: "2026-06-11 -- Quick task 260611-ozt: split publish into standalone push-only publish.yml (kills orphaned CI / publish (pull_request) pending status, WR-01); release model documented in README + publish.yml. Branch-protection contexts unchanged." progress: - total_phases: 16 + total_phases: 17 completed_phases: 2 total_plans: 8 completed_plans: 8 @@ -201,9 +201,9 @@ Recent decisions affecting current work: ## Session Continuity -Last session: 2026-06-11T22:00:00.000Z -Stopped at: Phase 08 complete — CI-01 + CI-02 delivered, publish job verified, SUMMARY + VERIFICATION written -Resume file: None — start Phase 09 with /gsd-plan-phase 9 +Last session: 2026-06-11T22:29:04.067Z +Stopped at: Phase 13 context gathered +Resume file: .planning/phases/13-real-lint-gate-eslint/13-CONTEXT.md ## Operator Next Steps -- 2.54.0 From ef87a3edf698463d2420c4c8de12672d08b8c537 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 11 Jun 2026 19:03:11 -0400 Subject: [PATCH 03/24] =?UTF-8?q?docs(13):=20research=20phase=20=E2=80=94?= =?UTF-8?q?=20ESLint=20flat=20config=20+=20Prettier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../13-real-lint-gate-eslint/13-RESEARCH.md | 672 ++++++++++++++++++ 1 file changed, 672 insertions(+) create mode 100644 .planning/phases/13-real-lint-gate-eslint/13-RESEARCH.md diff --git a/.planning/phases/13-real-lint-gate-eslint/13-RESEARCH.md b/.planning/phases/13-real-lint-gate-eslint/13-RESEARCH.md new file mode 100644 index 0000000..306ecdb --- /dev/null +++ b/.planning/phases/13-real-lint-gate-eslint/13-RESEARCH.md @@ -0,0 +1,672 @@ +# Phase 13: Real Lint Gate (ESLint) - Research + +**Researched:** 2026-06-11 +**Domain:** ESLint flat config + typescript-eslint type-aware linting + Prettier — pnpm monorepo +**Confidence:** MEDIUM + +--- + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions +- **D-13-01:** Use typescript-eslint `recommendedTypeChecked` (type-aware), not the non-type-aware `recommended`. Enable via `projectService: true`. +- **D-13-02:** React + react-hooks plugins for `apps/pwa` only; `apps/api` is Node/TS only (no React config). +- **D-13-03:** Do NOT adopt `strict`/`strictTypeChecked` presets — too much churn on the existing codebase. +- **D-13-04:** Run with `--max-warnings 0` — any warning fails CI. +- **D-13-05:** Fix all violations now. Phase is not done until `pnpm lint` and `pnpm format:check` are green. +- **D-13-06 (HARD CONSTRAINT):** Fixes must address the violation, not mask it. No blanket `eslint-disable`. Any `eslint-disable-next-line` requires a justifying inline comment. A type-aware finding is a candidate bug — review before suppressing. +- **D-13-07:** Prettier + standalone `prettier --check` CI step (separate from lint), AND `eslint-config-prettier` in flat config to disable conflicting formatting rules. `eslint-plugin-prettier` is rejected. +- **D-13-08:** All files get reformatted — isolated reformat commit before logic-fix commits for reviewability. +- **D-13-09:** Lint ALL TS/TSX: app `src/`, vitest tests, Playwright e2e specs, and config files. +- **D-13-10:** Config files that `projectService` cannot type-check need a dedicated `disableTypeChecked` override block. + +### Claude's Discretion +- Flat-config file layout (single root `eslint.config.js` vs per-app configs). +- Exact Prettier options (`.prettierrc`) — standard defaults; no bikeshedding. +- CI step ordering within `fast-checks` (lint → format:check → typecheck → tests). + +### Deferred Ideas (OUT OF SCOPE) +- None. + + +--- + +## Summary + +Phase 13 installs ESLint (v9 flat config) + typescript-eslint (v8, `recommendedTypeChecked`, `projectService: true`) across both apps, adds React/react-hooks plugins scoped to the PWA, wires package-level `lint` scripts so the existing CI slot activates, adds Prettier with a standalone `format:check` CI step, and fixes every first-run violation. + +**Critical version constraint:** `eslint-plugin-react@7.37.5` declares peer dependency `"eslint": "^3 || ... || ^9.7"` — it does not yet support ESLint 10. A GitHub issue (jsx-eslint/eslint-plugin-react#3977) confirms a runtime API incompatibility with ESLint 10 (`contextOrFilename.getFilename is not a function`). Pin ESLint to v9.x (maintenance release `9.39.4`) to stay compatible with both plugins. `typescript-eslint@8.x` supports `^8.57.0 || ^9.0.0 || ^10.0.0`, so it is compatible with ESLint 9. + +**Primary recommendation:** Single root `eslint.config.js` with per-glob overrides; all lint dependencies installed as root `devDependencies`; PWA React/hooks plugins applied only to `apps/pwa/**` globs; config/non-project files handled with `tseslint.configs.disableTypeChecked` override; `prettier --check` as a separate CI step after `pnpm lint`. + +--- + +## Architectural Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|-------------|----------------|-----------| +| ESLint config authoring | Root (dev-time) | Per-app overrides | Single config controls all workspaces; per-app globs scope plugins | +| Type-aware linting | Build-time / CI | Local dev | Requires TypeScript program; TS is already available at build time | +| Prettier formatting | Root (dev-time) | CI gate | `format` runs locally; `format:check` runs in CI | +| CI lint gate | fast-checks job | — | `pnpm lint` already present; activates when package scripts exist | +| CI format gate | fast-checks job | — | New `pnpm format:check` step added after `pnpm lint` | + +--- + +## Standard Stack + +### Core Packages +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| `eslint` | `9.39.4` (pin to 9.x) | Core linter | ESLint 9.x — last stable before ESLint 10 breaks plugin compat | +| `@eslint/js` | `9.39.4` | JS recommended rules config | Peer of ESLint; same version | +| `typescript-eslint` | `8.61.0` | TS parser + plugins + preset configs | Official combined package; supports ESLint ^9 | +| `eslint-plugin-react` | `7.37.5` | React-specific rules | PWA only; 47M downloads/wk | +| `eslint-plugin-react-hooks` | `7.1.1` | Hooks rules (`rules-of-hooks`, `exhaustive-deps`) | PWA only; official React team plugin; 80M downloads/wk | +| `eslint-config-prettier` | `10.1.8` | Disables ESLint formatting rules that conflict with Prettier | MUST be last in flat config array | +| `prettier` | `3.8.4` | Code formatter | Standalone; paired with `eslint-config-prettier` | + +**Version verification:** +```bash +# Verified via npm registry 2026-06-11: +# eslint 9.39.4 (maintenance tag; 10.4.1 is latest but breaks react plugin) +# @eslint/js 9.39.4 (matches eslint version) +# typescript-eslint 8.61.0 +# eslint-plugin-react 7.37.5 +# eslint-plugin-react-hooks 7.1.1 +# eslint-config-prettier 10.1.8 +# prettier 3.8.4 +``` + +### Installation + +Install all lint/format packages as root workspace `devDependencies`: + +```bash +# From repo root +pnpm add -D -w \ + eslint@9.39.4 \ + "@eslint/js@9.39.4" \ + typescript-eslint@8.61.0 \ + eslint-plugin-react@7.37.5 \ + eslint-plugin-react-hooks@7.1.1 \ + eslint-config-prettier@10.1.8 \ + prettier@3.8.4 +``` + +Installing at root (not per-app) is the idiomatic approach for a small 2-app pnpm workspace: all packages share one node_modules resolution, and the single root `eslint.config.js` can import every plugin without cross-package symlink complexity. + +### Alternatives Considered +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| ESLint 9.39.4 (pin) | ESLint 10.4.1 | 10.x breaks `eslint-plugin-react` — open compat issue; unblocked when plugin releases fix | +| `recommendedTypeChecked` | `strictTypeChecked` | `strict` adds ~15 more rules; D-13-03 explicitly rejects the extra churn | +| root devDependencies | per-app devDependencies | Per-app adds config import complexity; root is idiomatic for shared dev tooling | +| `eslint-plugin-react` + `react-hooks` | `react-hooks` alone | `react-hooks` covers hook rules; `eslint-plugin-react` adds JSX/prop-types/display-name rules worth having for a production PWA | + +--- + +## Package Legitimacy Audit + +| Package | Registry | Age | Downloads | Source Repo | Verdict | Disposition | +|---------|----------|-----|-----------|-------------|---------|-------------| +| `eslint` | npm | 14 yrs | 132M/wk | github.com/eslint/eslint | SUS (too-new publish) | Approved — canonical project, official github.com/eslint/eslint repo confirmed | +| `@eslint/js` | npm | 3 yrs | 117M/wk | github.com/eslint/eslint | OK | Approved | +| `typescript-eslint` | npm | 6 yrs | 69M/wk | github.com/typescript-eslint/typescript-eslint | SUS (too-new publish) | Approved — canonical official monorepo confirmed | +| `eslint-plugin-react` | npm | 11 yrs | 47M/wk | github.com/jsx-eslint/eslint-plugin-react | OK | Approved | +| `eslint-plugin-react-hooks` | npm | 6 yrs | 80M/wk | github.com/facebook/react | OK | Approved | +| `eslint-config-prettier` | npm | 8 yrs | 56M/wk | github.com/prettier/eslint-config-prettier | OK | Approved | +| `prettier` | npm | 9 yrs | 108M/wk | github.com/prettier/prettier | SUS (too-new publish) | Approved — canonical official repo confirmed | + +**Packages removed due to SLOP verdict:** None. +**Packages flagged as suspicious (SUS):** `eslint`, `typescript-eslint`, `prettier` — flagged only because each received a new release within the last few weeks (the legitimacy seam's `too-new` signal). All three have canonical GitHub repository URLs matching the well-known official projects and download counts in the tens/hundreds of millions. No postinstall scripts. Approved for use without additional human verification checkpoint. + +--- + +## Architecture Patterns + +### System Architecture Diagram + +``` +repo root +├── eslint.config.js ← single flat config file (ESM) +│ ├── ignores block ← dist/, node_modules/, migrations/ +│ ├── base block ← js.recommended + tseslint.recommendedTypeChecked +│ │ languageOptions.parserOptions.projectService: true +│ │ tsconfigRootDir: import.meta.dirname +│ ├── pwa-react block ← files: apps/pwa/**/*.{ts,tsx} +│ │ extends: [reactPlugin.configs.flat.recommended, +│ │ reactHooks.configs.flat.recommended] +│ │ settings.react.version: detect +│ └── config-files block ← files: [*.config.ts, apps/*/*.config.*] +│ extends: [tseslint.configs.disableTypeChecked] +│ +├── .prettierrc ← minimal config (defaults) +├── .prettierignore ← dist, node_modules, pnpm-lock, migrations +│ +├── package.json root scripts: +│ "lint": "pnpm -r --if-present lint" +│ "format": "prettier --write ." +│ "format:check": "prettier --check ." +│ +├── apps/api/package.json: +│ "lint": "eslint src/ tests/" ← triggers via pnpm -r lint +│ +└── apps/pwa/package.json: + "lint": "eslint src/ e2e/" ← triggers via pnpm -r lint +``` + +``` +CI fast-checks job (ci.yml): + pnpm lint ← pnpm -r --if-present lint → runs api lint then pwa lint + pnpm format:check ← prettier --check . (NEW step, after lint) + pnpm typecheck ← unchanged + PWA unit tests ← unchanged +``` + +### Recommended Project Structure +``` +repo root +├── eslint.config.js # single flat config (ESM); type: module at root +├── .prettierrc # minimal JSON config +├── .prettierignore # excludes for Prettier +├── package.json # add format / format:check scripts +└── apps/ + ├── api/ + │ └── package.json # add: "lint": "eslint src/ tests/ --max-warnings 0" + └── pwa/ + └── package.json # add: "lint": "eslint src/ e2e/ --max-warnings 0" +``` + +### Pattern 1: `projectService: true` with Multiple tsconfigs (Monorepo) + +**What:** `projectService: true` is the typescript-eslint v8 replacement for `parserOptions.project`. It auto-discovers and loads all tsconfig.json files in the repository — no manual glob list required. For a 2-app pnpm workspace it handles `apps/api/tsconfig.json`, `apps/pwa/tsconfig.json`, and `apps/pwa/tsconfig.e2e.json` automatically. [CITED: typescript-eslint.io/troubleshooting/typed-linting/monorepos/] + +**When to use:** Any monorepo using typescript-eslint v8 — projectService is the preferred approach. + +```js +// eslint.config.js +// Source: typescript-eslint.io/getting-started/typed-linting/ +import js from '@eslint/js' +import tseslint from 'typescript-eslint' +import reactPlugin from 'eslint-plugin-react' +import reactHooks from 'eslint-plugin-react-hooks' +import prettierConfig from 'eslint-config-prettier/flat' + +export default tseslint.config( + // ── Global ignores (replaces .eslintignore) ────────────────────────────── + { + ignores: [ + '**/dist/**', + '**/node_modules/**', + '**/src/db/migrations/**', // generated Drizzle SQL files + 'pnpm-lock.yaml', + ], + }, + + // ── Base: all TS/TSX files in both apps ────────────────────────────────── + { + files: ['apps/**/*.{ts,tsx}'], + extends: [ + js.configs.recommended, + tseslint.configs.recommendedTypeChecked, + ], + languageOptions: { + parserOptions: { + projectService: true, + tsconfigRootDir: import.meta.dirname, + }, + }, + }, + + // ── React + Hooks: PWA only ─────────────────────────────────────────────── + { + files: ['apps/pwa/**/*.{ts,tsx}'], + extends: [ + reactPlugin.configs.flat.recommended, + reactHooks.configs.flat.recommended, + ], + settings: { + react: { version: 'detect' }, + }, + }, + + // ── Config files: disable type-aware rules ──────────────────────────────── + // These files are not included in any tsconfig project (they are tool configs + // consumed by drizzle-kit, vite, vitest, playwright — not by tsc compilation). + // projectService cannot type-check them; using disableTypeChecked avoids the + // "file was not found in any of the provided project(s)" error. + // Source: typescript-eslint.io/troubleshooting/typed-linting/#i-get-errors-telling-me-the-file-must-be-included-in-at-least-one-of-the-projects + { + files: [ + 'apps/api/drizzle.config.ts', + 'apps/api/vitest.config.ts', + 'apps/pwa/vite.config.ts', + 'apps/pwa/vitest.config.ts', + 'apps/pwa/playwright.config.ts', + ], + extends: [tseslint.configs.disableTypeChecked], + }, + + // ── eslint-config-prettier: MUST BE LAST ───────────────────────────────── + // Disables all ESLint formatting rules that conflict with Prettier. + // Source: github.com/prettier/eslint-config-prettier + prettierConfig, +) +``` + +**Critical note on `reactPlugin.configs.flat.recommended`:** As of eslint-plugin-react@7.37.5, the flat config export is `reactPlugin.configs.flat.recommended` (not `reactPlugin.configs['flat/recommended']`). [CITED: github.com/jsx-eslint/eslint-plugin-react] The plugin must be imported under the standard name `react` — not aliased — or `eslint-config-prettier` won't be able to disable its formatting rules correctly. [CITED: github.com/prettier/eslint-config-prettier] + +**Note on `reactHooks.configs.flat.recommended`:** As of eslint-plugin-react-hooks@7.1.1, the flat config export is `reactHooks.configs.flat.recommended`. The `recommended-latest` variant enables experimental React Compiler rules — do not use for production. [ASSUMED: based on npm registry inspection + community docs] + +### Pattern 2: `--max-warnings 0` in package `lint` scripts + +Per D-13-04, every violation must be either an error or off — no non-blocking warnings: + +```json +// apps/api/package.json +"lint": "eslint src/ tests/ --max-warnings 0" + +// apps/pwa/package.json +"lint": "eslint src/ e2e/ --max-warnings 0" +``` + +The root `pnpm -r --if-present lint` then activates both. Note: the API's `tsconfig.json` excludes `tests/` from its `include`, which means test files are outside the tsconfig project. The `disableTypeChecked` config-files override above covers named config files; to also cover test files from a different project, ensure the tests directory is reachable via a tsconfig (the API vitest config uses `globals: true` and the tsconfig includes `"types": ["vitest/globals"]` — but the tsconfig exclude: ["tests"] means those files are NOT in the project). See Pitfall 3 for the required solution. + +### Pattern 3: Prettier Standalone Gate + +```json +// root package.json — add to scripts: +"format": "prettier --write .", +"format:check": "prettier --check ." +``` + +```yaml +# .gitea/workflows/ci.yml — add after Lint step, before Typecheck: +- name: Format check + run: pnpm format:check +``` + +### Anti-Patterns to Avoid +- **`void promise` to silence `no-floating-promises`:** `void foo()` is a mask — ESLint `no-floating-promises` accepts `void` as a suppression. The rule is flagging a genuinely unhandled promise. Use `.catch()` or `await`. Only use `void` when the promise is truly fire-and-forget AND that is documented explicitly. +- **Using `eslint-plugin-prettier`:** Runs Prettier as an ESLint rule — formats files twice, produces noisy diff output. Prettier docs explicitly recommend against this. The correct split is `eslint-config-prettier` (disables conflicting rules in ESLint) + `prettier --check` (standalone format gate). [CITED: prettier.io/docs/en/integrating-with-linters] +- **`allowDefaultProject` for config files:** While it works, it is fragile (glob resolution issues in some configurations). The `disableTypeChecked` override on specific config file globs is simpler and explicit. [CITED: typescript-eslint.io/blog/project-service] +- **Aliasing plugins:** `plugins: { ts: typescriptEslint }` instead of `{ '@typescript-eslint': tseslint.plugin }` — `eslint-config-prettier` can't disable its rules. Always use the canonical plugin name. +- **ESLint `--cache` with type-aware linting:** Type-aware linting is incompatible with ESLint's file cache (`--cache`). The cache cannot track TypeScript program state. Do not use `--cache` when `projectService: true` is active. [CITED: github.com/typescript-eslint/typescript-eslint/issues/4694] + +--- + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Disabling formatting rules | Manual rule-off list | `eslint-config-prettier` | Prettier maintainers curate the list; it covers @typescript-eslint formatting rules too | +| TypeScript project resolution in monorepo | Manual tsconfig glob list | `projectService: true` | v8 projectService auto-discovers all tsconfigs; no maintenance overhead | +| React hooks exhaustive-deps enforcement | Manual code review | `eslint-plugin-react-hooks` | Statically catches missing dependencies the type system can't | +| Non-project file handling | `tsconfig.eslint.json` with `include: ['**/*']` | `disableTypeChecked` override | Simpler; avoids creating a catch-all tsconfig that degrades type checking | + +**Key insight:** The `eslint-config-prettier` package is the canonical way to integrate Prettier with ESLint. Without it, ESLint's `@typescript-eslint/indent` and similar formatting rules will conflict with Prettier's output, producing a feedback loop where `eslint --fix` and `prettier --write` undo each other. + +--- + +## Common Pitfalls + +### Pitfall 1: ESLint 10 + eslint-plugin-react Runtime Error +**What goes wrong:** Installing `eslint@latest` (10.x) with `eslint-plugin-react@7.37.5` throws `TypeError: contextOrFilename.getFilename is not a function` at lint time. +**Why it happens:** ESLint 10 removed legacy Context API methods that `eslint-plugin-react`'s version-detection utility still calls. +**How to avoid:** Pin `eslint@9.39.4` (the maintenance tag). The plugin issue is tracked at jsx-eslint/eslint-plugin-react#3977 and is unresolved as of 2026-06-11. +**Warning signs:** pnpm install succeeds but `pnpm lint` immediately throws a TypeError before reporting any actual lint errors. + +### Pitfall 2: API test files excluded from tsconfig project +**What goes wrong:** `apps/api/tsconfig.json` has `"exclude": ["tests"]`. ESLint with `projectService: true` cannot type-check `apps/api/tests/*.ts` — you'll get "The file does not match your project config" errors, or type-aware rules silently won't fire on test files. +**Why it happens:** `projectService` discovers tsconfig.json and respects its `exclude`. Test files outside the project scope can't receive type-aware linting. +**How to avoid:** Add a dedicated `disableTypeChecked` override block in `eslint.config.js` for `apps/api/tests/**/*.ts`. Those files will still be linted with non-type-aware rules (syntax, `no-unused-vars`, etc.) — just not the type-aware ones. Alternatively, create an `apps/api/tsconfig.test.json` that includes the tests and verify `projectService` picks it up. +**Warning signs:** "Parsing error: ESLint was configured to run on `apps/api/tests/foo.test.ts` but that file was not found in any of the provided project(s)." + +### Pitfall 3: `vitest.config.ts` / `drizzle.config.ts` not in any tsconfig project +**What goes wrong:** Config files (`vite.config.ts`, `drizzle.config.ts`, `vitest.config.ts`, `playwright.config.ts`) sit outside the `include` arrays of all tsconfigs. Type-aware linting on them produces "file must be included in at least one of the projects" errors. +**Why it happens:** These files are tool configs consumed by drizzle-kit / Vite / Vitest / Playwright, not by the TypeScript compiler. The tsconfigs correctly exclude them. +**How to avoid:** Add these to the `disableTypeChecked` override block in `eslint.config.js` (see Pattern 1 skeleton above). They receive non-type-aware rules only. +**Warning signs:** Lint errors only on config files, none on src/ — and the error references the project inclusion issue. + +### Pitfall 4: `no-floating-promises` on setInterval callbacks +**What goes wrong:** The broker workers (`outboxWorker.ts`, `reminderScheduler.ts`, `poller.ts`) use the pattern: +```ts +setInterval(() => { + runOutboxDrain().catch((err: unknown) => { ... }) +}, 15_000) +``` +`setInterval` accepts `() => void`, so the arrow function IS void-returning (the `.catch()` return value is discarded). This pattern is correct and ESLint `no-floating-promises` should NOT fire here because the promise is handled via `.catch()`. +**Why it matters:** If any worker uses `runX()` without `.catch()`, that IS a real floating promise that should be fixed (not suppressed). +**How to avoid:** The `.catch()` on every worker's inner async call is already the correct fix. Verify no worker uses the bare pattern `setInterval(() => { runX() }, ...)` without `.catch()`. +**Warning signs:** If the rule fires on a `.catch()`-chained promise, the rule is incorrectly applied — check that the `.catch()` callback is typed as `(err: unknown) => void`. + +### Pitfall 5: `no-misused-promises` on event listener callbacks +**What goes wrong:** Pattern like `document.addEventListener('click', async () => { ... })` — the event listener callback is typed to return void but the async function returns `Promise`. `no-misused-promises` flags this as a misuse. +**Why it happens:** The type system marks `addEventListener` callbacks as void-returning, so passing an async function is technically unsound. +**How to avoid:** Wrap the async logic: `document.addEventListener('click', () => { void asyncHandler() })` or extract to a named function. The `void` operator here is legitimate: it explicitly signals "I know this is a promise, I'm not awaiting it, and I accept that responsibility." +**Warning signs:** Violations on event listener registrations in React `useEffect` hooks or service worker event handlers. + +### Pitfall 6: `sw.ts` — service worker file type context +**What goes wrong:** `apps/pwa/src/sw.ts` declares `/// ` and `declare const self: ServiceWorkerGlobalScope`. The PWA tsconfig has `"lib": ["ES2023", "DOM", "DOM.Iterable"]`, which includes DOM but NOT ServiceWorker APIs. The SW file uses a separate lib reference to access SW types. ESLint may flag missing types for SW-specific globals. +**Why it happens:** The SW file is in `apps/pwa/src/` which is in the PWA tsconfig's `include`. The webworker lib reference provides the types but projectService may need to see this correctly. +**How to avoid:** Ensure the SW file is parsed under the correct tsconfig. If type errors arise specifically on `sw.ts`, consider adding it to a SW-specific `disableTypeChecked` override rather than fighting the lib reference. + +### Pitfall 7: `eslint-config-prettier` import path in ESM flat config +**What goes wrong:** `import eslintConfigPrettier from 'eslint-config-prettier'` does NOT work in ESM flat config — it returns the full package with all sub-configs. The correct import for ESM flat config is `from 'eslint-config-prettier/flat'`. +**Why it happens:** `eslint-config-prettier` exports different shapes for CJS legacy config vs ESM flat config. +**How to avoid:** Always use `eslint-config-prettier/flat` in ESM `eslint.config.js`. [CITED: github.com/prettier/eslint-config-prettier#readme] +**Warning signs:** `prettierConfig` resolves to an object with extra properties that don't match the flat config schema. + +### Pitfall 8: `React` import in main.tsx and ErrorBoundary.tsx +**What goes wrong:** `eslint-plugin-react` with `react/jsx-runtime` preset or the `react/react-in-jsx-scope` rule will report `React` as unused when JSX transform is used. But in this codebase, `main.tsx` uses `` (requires the `React` namespace) and `ErrorBoundary.tsx` extends `React.Component` (also uses the namespace directly). These are legitimate uses — the rule should NOT fire. +**Why it happens:** The `react/react-in-jsx-scope` rule is disabled in the `flat.recommended` config since React 17+ JSX transform. But `React.StrictMode` / `React.Component` still require the import. +**How to avoid:** No action needed — `reactPlugin.configs.flat.recommended` already disables `react/react-in-jsx-scope`. The `React` import stays because it IS used (namespace access, not just JSX). + +--- + +## First-Run Violations: Expected Findings and Correct Fixes + +This section maps likely first-run violations to the correct fix per D-13-05/D-13-06. + +### `@typescript-eslint/no-unsafe-*` family (no-unsafe-member-access, no-unsafe-argument, no-unsafe-assignment) + +**Likely locations:** +- `apps/api/src/broker/sync.ts` — `ical.js` returns values typed as `any` via `getFirstPropertyValue()`. Multiple `as string` casts on the returned values. +- `apps/api/src/broker/expand.ts` — similar ical.js property access. +- `apps/pwa/src/sw.ts` — `event.data.json() as Record` — already correctly cast. +- `apps/pwa/src/components/EventForm.tsx:275` — `(occurrence as any)?.recurrence` — the deliberate `as any`. + +**Correct fix:** Add type predicates or narrow via `typeof`/`instanceof` checks before accessing properties. For ical.js specifically (a library with weak typings), a targeted `eslint-disable-next-line @typescript-eslint/no-unsafe-member-access // ical.js returns untyped property values` with justification comment is acceptable — this is a known external library typing limitation, not a bug. + +**Distinction real-fix vs mask:** +- MASK: `// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment` with no explanation. +- REAL FIX OR JUSTIFIED SUPPRESS: `// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access // ical.js getFirstPropertyValue() returns 'any'; the caller immediately validates the value`. + +### `@typescript-eslint/no-floating-promises` + +**Likely locations:** +- Broker workers: all three `setInterval` blocks already use `.catch()`. The pattern `runX().catch(...)` should NOT trigger `no-floating-promises` because the promise is handled. +- `apps/api/src/broker/outboxWorker.ts:237` — `dispatchEventChange(change, userId).catch(...)` — already handled. + +**If the rule fires on a `.catch()` chain:** The return of `.catch()` is also a Promise. If the outer function doesn't return it, the chained promise itself is floating. In a `setInterval` callback that returns `void`, this is fine — but the rule may need the callback pattern `() => { runX().catch(...) }` to be recognized as void (not `() => runX().catch(...)`). Adjust by wrapping in `void`: +```ts +setInterval(() => { + void runOutboxDrain().catch((err: unknown) => { ... }) +}, 15_000) +``` +The explicit `void` operator here is a legitimate signal: "I know this returns a promise and I am intentionally not awaiting it because the setInterval schedule handles the next tick." + +### `@typescript-eslint/require-await` + +**What it catches:** Async functions that contain no `await` expression. +**Likely location:** Any route handler or utility that was written async for consistency but doesn't actually await. +**Correct fix:** Remove `async` keyword if the function returns a non-Promise value, OR actually `await` the operation if the async was intended. + +### `@typescript-eslint/no-unused-vars` + +**What it catches:** Declared but never read variables/imports. +**Convention:** The `_` prefix convention for intentionally unused params: `(_unusedParam: string) => ...`. Configure the rule to ignore `_`-prefixed names: +```js +rules: { + '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }], +} +``` +**Likely locations:** Destructured params in error handlers (`catch (err) { }` — empty catches don't have a binding, so no violation). React test files that import React explicitly. + +### `react/display-name` + +**What it catches:** React components defined without a displayName (typically forwardRef or memo-wrapped components). +**Correct fix:** Either add a displayName or use named function expressions (`const MyComp = memo(function MyComp() { ... })`). + +### `@typescript-eslint/no-explicit-any` (from `recommendedTypeChecked`) + +**Status:** `no-explicit-any` is in `recommended` (not type-aware). It will flag explicit `any` annotations. +**Specific case — `EventForm.tsx:275`:** `(occurrence as any)?.recurrence` — deliberate cast because `occurrence` comes from ical.js expansion with a weak type. This is a justified suppress: the ical.js types don't expose `recurrence` on occurrence objects, and fixing requires a proper type guard. A targeted disable with justification is acceptable here. + +--- + +## Prettier Configuration + +### `.prettierrc` + +Standard defaults — no bikeshedding per CLAUDE.md: + +```json +{ + "semi": true, + "singleQuote": true, + "tabWidth": 2, + "trailingComma": "all", + "printWidth": 100 +} +``` + +**Note on `singleQuote`:** The existing codebase uses single quotes throughout (visible in all source files). Setting `singleQuote: true` ensures Prettier matches the existing style rather than converting everything to double quotes (which would produce a massive diff even after the first reformat commit). [ASSUMED — based on codebase inspection] + +### `.prettierignore` + +``` +dist/ +node_modules/ +pnpm-lock.yaml +apps/api/src/db/migrations/ +*.html +``` + +Drizzle-generated SQL migration files are excluded — they are generated artefacts with specific formatting. + +### `eslint-config-prettier` placement + +Must be the LAST config object in the `tseslint.config(...)` array. Any config that comes after it could re-enable formatting rules. [CITED: github.com/prettier/eslint-config-prettier#readme] + +```js +// eslint.config.js — the import: +import prettierConfig from 'eslint-config-prettier/flat' // flat config import path + +// In the config array — ALWAYS last: +export default tseslint.config( + // ... all other configs ... + prettierConfig, // LAST +) +``` + +--- + +## CI Wiring + +### Existing CI structure (`fast-checks` job in `.gitea/workflows/ci.yml`) + +```yaml +# Current order: +- name: Lint # runs pnpm lint → pnpm -r --if-present lint → activates once package scripts exist +- name: Typecheck # unchanged +- name: PWA unit tests +``` + +### Required CI change — add Format Check step + +```yaml +- name: Lint + run: pnpm lint + +- name: Format check # ← NEW step (D-13-07) + run: pnpm format:check + +- name: Typecheck + run: pnpm typecheck + +- name: PWA unit tests + run: pnpm --filter @familysync/pwa test +``` + +This ordering is deliberate: lint violations are the most informative (they point to code issues), format violations are mechanical, and typecheck/tests come last as they are slower. + +### Why `pnpm lint` automatically activates + +The root `package.json` already has `"lint": "pnpm -r --if-present lint"`. The `--if-present` flag means it currently exits 0 when no package defines a `lint` script. Once the `apps/api/package.json` and `apps/pwa/package.json` each add a `lint` script, this step becomes real without any CI change. + +### Lint architecture: root invocation or per-package? + +**Recommendation:** Root `pnpm -r --if-present lint` (the existing wiring). Each package runs `eslint --max-warnings 0`. The root `eslint.config.js` is shared, but each invocation is scoped to that package's directory. + +**Why not a single top-level `eslint apps/` invocation?** A single invocation would work with projectService, but would require adding it as a root-level script separate from the existing `pnpm -r` wiring. The per-package approach mirrors the existing `typecheck` pattern and keeps each app's lint independently runnable. + +--- + +## Performance + +**Type-aware linting time estimate:** For a project of this size (~91 source files + tests + e2e ≈ ~130 total TS/TSX files), type-aware lint time is approximately 2-5× tsc compile time. The tsc compile for both apps together is typically 15-30 seconds. Estimated lint time: 30-90 seconds CI runtime. [ASSUMED — based on community benchmarks; no direct measurement] + +**No cache in CI (D-PROBE-04):** ESLint's `--cache` is incompatible with type-aware linting (the cache can't track TypeScript program state changes). Even if `actions/cache` worked on this runner, the ESLint file cache would be unusable. Do not use `--cache` with type-aware lint. [CITED: github.com/typescript-eslint/typescript-eslint/issues/4694] + +**projectService vs `project` option performance:** `projectService` is generally faster than `parserOptions.project` with globs because it uses TypeScript's Language Service API with incremental compilation support. [CITED: typescript-eslint.io/blog/project-service] + +**Acceptable for CI:** Given that `actions/cache` is already omitted from the fast-checks job (D-PROBE-04), and `pnpm install` without cache takes ~30s, an additional 30-90s for type-aware lint is acceptable in the CI pipeline. The entire fast-checks job currently runs lint (0s no-op) + typecheck (~30s) + PWA tests (~45s). Adding lint will increase the job duration but it runs in parallel with the `api` and `harness` jobs, so it does not extend the overall PR wall time. + +--- + +## Runtime State Inventory + +> Greenfield lint config — no renaming or migration. Section omitted per phase type. + +--- + +## Environment Availability + +| Dependency | Required By | Available | Version | Fallback | +|------------|------------|-----------|---------|----------| +| Node.js 22 | ESLint, typescript-eslint | ✓ | 22.x (CI: actions/setup-node@v4) | — | +| TypeScript 5.x | type-aware linting | ✓ | ^5.5.0 (both apps devDeps) | — | +| pnpm 11.5.1 | monorepo install | ✓ | 11.5.1 | — | +| npm registry | package install | ✓ | — | — | + +**Missing dependencies with no fallback:** None. +**Missing dependencies with fallback:** None. + +--- + +## Validation Architecture + +### Test Framework + +| Property | Value | +|----------|-------| +| Framework | Vitest (apps/api: node env; apps/pwa: jsdom env) | +| Config file | `apps/api/vitest.config.ts`, `apps/pwa/vitest.config.ts` | +| Quick run (api) | `pnpm --filter @familysync/api test` | +| Quick run (pwa) | `pnpm --filter @familysync/pwa test` | + +### Phase Requirements → Test Map + +| Behavior | Test Type | Automated Command | Notes | +|----------|-----------|-------------------|-------| +| `pnpm lint` exits non-zero on a deliberate violation | Smoke gate | Introduce a test file with a known violation, run lint, assert non-zero exit | Manual CI run required; not a vitest test | +| `pnpm format:check` exits non-zero on unformatted file | Smoke gate | Stage an unformatted file, run `prettier --check`, assert non-zero | Manual verification | +| CI lint step activates (not a no-op) | Integration | PR to main; observe CI step now reports violations (not silent pass) | CI run | +| All first-run violations fixed — both apps lint green | End-to-end gate | `pnpm lint` exits 0 | Must pass before phase complete | +| Prettier reformats files | End-to-end gate | `pnpm format && git diff --stat` shows changes | Visual inspection | +| `pnpm format:check` green after reformat | End-to-end gate | `pnpm format:check` exits 0 | Must pass before phase complete | + +### Deliberate-Violation Test (ROADMAP success criterion 1) + +**What:** Introduce a temporary deliberate lint violation to prove the gate actually fails CI (not just passes silently). This is the ROADMAP's explicit first success criterion. +**Where:** Create a throwaway file `apps/api/src/_lint-gate-test.ts` containing: +```ts +// eslint-gate-test: proves no-floating-promises fires +export function testFloatingPromise(): void { + Promise.resolve(1) // deliberately unhandled — should trigger no-floating-promises +} +``` +**Process:** +1. Install + config committed; run `pnpm lint` locally — should exit non-zero with violation reported. +2. Delete the test file; `pnpm lint` exits 0. +3. Commit without the test file. + +### Wave 0 Gaps +- No new vitest test files are needed for this phase — the validation is the lint gate itself. +- The "deliberate violation proves gate fails" check is a manual one-time smoke test, not an automated spec. + +--- + +## Security Domain + +> `security_enforcement: true` is set in `.planning/config.json`. ASVS Level 1 applies. + +### Applicable ASVS Categories + +| ASVS Category | Applies | Standard Control | +|---------------|---------|-----------------| +| V2 Authentication | No | Not touched by this phase | +| V3 Session Management | No | Not touched | +| V4 Access Control | No | Not touched | +| V5 Input Validation | Indirectly | `no-unsafe-*` rules enforce type-safe access to external data | +| V6 Cryptography | No | Not touched | + +### Known Threat Patterns for This Phase + +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|---------------------| +| Masking a real bug with `eslint-disable` | Tampering (code quality) | D-13-06 constraint: no blanket disables; justification required | +| Floating promise swallowing errors silently | Information Disclosure | `no-floating-promises` catches unhandled rejections that may hide security-relevant errors | +| `no-unsafe-*` on user-controlled data paths | Tampering | Use proper type guards instead of `as any` casts on data from external sources | + +**Note:** This phase primarily improves code quality, not security posture directly. The security value is indirect: `no-floating-promises` and `no-unsafe-*` rules prevent the class of bugs (unhandled rejections, unsafe data access) that could propagate into security-relevant code paths. + +--- + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| A1 | `reactHooks.configs.flat.recommended` is the correct flat config export path for eslint-plugin-react-hooks@7.1.1 | Architecture Patterns | Lint config fails to load; need to use manual plugin registration instead | +| A2 | Prettier `.prettierrc` `singleQuote: true` matches the codebase's existing style | Prettier Configuration | Reformat commit will be significantly larger; may need `singleQuote: false` | +| A3 | Type-aware lint adds 30-90 seconds to CI fast-checks job | Performance | Could be faster (no impact) or slower (may need investigation) | +| A4 | `apps/api/src/broker/sync.ts` ical.js casts will trigger `no-unsafe-*` violations | First-Run Violations | May be fewer violations than expected; or may need targeted disables | + +--- + +## Open Questions + +1. **API tests outside tsconfig project** + - What we know: `apps/api/tsconfig.json` excludes `tests/`. API tests are in `apps/api/tests/` (26 files, not `src/tests/`). + - What's unclear: Does `projectService` fail hard on those files, or does it fall back gracefully? If hard fail, should we add `disableTypeChecked` for `apps/api/tests/**` or create `apps/api/tsconfig.test.json`? + - Recommendation: Add `apps/api/tests/**/*.ts` to the `disableTypeChecked` override block in the initial config. If type-aware rules on test files are desired later, add a separate tsconfig.test.json. + +2. **`eslint-plugin-react` necessity** + - What we know: The PWA has 61 .ts/.tsx files. `react/display-name` and `react/prop-types` are the most common rules. React 19 makes `prop-types` moot (TypeScript props typing supersedes it). + - What's unclear: Is `eslint-plugin-react` worth the first-run violation churn vs just `react-hooks`? + - Recommendation: D-13-02 locks in React plugin for PWA. Disable `react/prop-types` explicitly (TypeScript handles this) to reduce first-run noise. + +--- + +## Sources + +### Primary (MEDIUM confidence — Context7 official docs) +- `/typescript-eslint/typescript-eslint` (Context7) — flat config, projectService, disableTypeChecked, monorepos +- `/websites/typescript-eslint_io` (Context7) — projectService blog post, allowDefaultProject +- `/prettier/eslint-config-prettier` (Context7) — flat config import path, plugin naming pitfall, placement +- `typescript-eslint.io/troubleshooting/typed-linting/monorepos/` — projectService requires no additional monorepo config +- `typescript-eslint.io/troubleshooting/typed-linting/performance/` — cache incompatibility, performance guidance + +### Secondary (LOW confidence — WebSearch verified against multiple sources) +- `github.com/jsx-eslint/eslint-plugin-react/issues/3977` — ESLint 10 runtime incompatibility (open issue confirmed) +- `github.com/facebook/react/issues/35758` — react-hooks ESLint 10 peerDep issue +- ESLint 9 maintenance release 9.39.4 confirmed via npm dist-tags +- eslint-plugin-react-hooks flat config export names confirmed via npm package inspection + community docs + +### Package Registry Verification +- All versions confirmed via `npm view version` on 2026-06-11 +- Repository URLs verified for all packages via `npm view --json` +- Postinstall scripts: none detected on any package + +## Metadata + +**Confidence breakdown:** +- Standard stack + versions: MEDIUM — confirmed via npm registry; ESLint 10 compat issue confirmed via GitHub issue +- Architecture: MEDIUM — based on official typescript-eslint docs via Context7 +- First-run violations: LOW-MEDIUM — based on codebase inspection; actual violations may differ + +**Research date:** 2026-06-11 +**Valid until:** 2026-07-11 (stable area; main risk is eslint-plugin-react releasing ESLint 10 support, which would allow unpinning) -- 2.54.0 From ee67dfecef8790af8ca2790cbf4c7341d48fdfb6 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 11 Jun 2026 19:04:13 -0400 Subject: [PATCH 04/24] docs(13): add research + validation strategy --- .../13-real-lint-gate-eslint/13-VALIDATION.md | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 .planning/phases/13-real-lint-gate-eslint/13-VALIDATION.md diff --git a/.planning/phases/13-real-lint-gate-eslint/13-VALIDATION.md b/.planning/phases/13-real-lint-gate-eslint/13-VALIDATION.md new file mode 100644 index 0000000..94a8758 --- /dev/null +++ b/.planning/phases/13-real-lint-gate-eslint/13-VALIDATION.md @@ -0,0 +1,82 @@ +--- +phase: 13 +slug: real-lint-gate-eslint +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-06-11 +--- + +# Phase 13 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. +> This phase's deliverable IS a validation gate (ESLint + Prettier). Most +> verification is gate-based (`pnpm lint` / `pnpm format:check` exit codes), +> not new vitest specs. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | Vitest (apps/api: node env; apps/pwa: jsdom env) — already present, unchanged | +| **Config file** | `apps/api/vitest.config.ts`, `apps/pwa/vitest.config.ts` | +| **Quick run command** | `pnpm lint` (root — runs ESLint across both apps) | +| **Full suite command** | `pnpm lint && pnpm format:check && pnpm typecheck && pnpm test` | +| **Estimated runtime** | ~30–60s (type-aware lint over ~91 src files + tests + e2e, no cache) | + +--- + +## Sampling Rate + +- **After every task commit:** Run `pnpm lint` (the gate under construction) +- **After every plan wave:** Run `pnpm lint && pnpm format:check` +- **Before `/gsd-verify-work`:** `pnpm lint` AND `pnpm format:check` AND `pnpm typecheck` AND `pnpm test` all green across both apps +- **Max feedback latency:** ~60 seconds + +--- + +## Per-Task Verification Map + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| 13-XX-XX | TBD | 1 | SC-1 (gate fails on violation) | — | N/A | smoke gate | `pnpm lint` exits non-zero on deliberate violation | ❌ W0 | ⬜ pending | +| 13-XX-XX | TBD | 1 | SC-1 (format gate fails) | — | N/A | smoke gate | `pnpm format:check` exits non-zero on unformatted file | ❌ W0 | ⬜ pending | +| 13-XX-XX | TBD | 2 | SC-3 (baseline green) | — | N/A | end-to-end gate | `pnpm lint` exits 0 (both apps) | ❌ W0 | ⬜ pending | +| 13-XX-XX | TBD | 2 | SC-3 (format baseline) | — | N/A | end-to-end gate | `pnpm format:check` exits 0 | ❌ W0 | ⬜ pending | +| 13-XX-XX | TBD | 2 | SC-2 (CI gate blocks PR) | — | N/A | integration | CI `fast-checks` lint + format:check steps report violations (not silent pass) | ❌ W0 | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky. Task IDs assigned by planner.* + +--- + +## Wave 0 Requirements + +- [ ] No new vitest test files are required — the validation is the lint/format gate itself. +- [ ] `eslint.config.js` + Prettier config + package-level `lint` scripts must exist before any gate assertion can run (this is the phase's own Wave 1 work, not a test scaffold). + +*Existing vitest infrastructure is untouched; this phase adds no unit specs.* + +--- + +## Manual-Only Verifications + +| Behavior | Requirement | Why Manual | Test Instructions | +|----------|-------------|------------|-------------------| +| Lint gate fails CI on a real violation | SC-1 | One-time smoke proof; not a repeatable automated spec | Create throwaway `apps/api/src/_lint-gate-test.ts` with an unhandled `Promise.resolve(1)` (triggers `no-floating-promises`); run `pnpm lint` → assert non-zero; delete file → assert exit 0; do NOT commit the throwaway file | +| Prettier reformats existing files | SC-3 | Visual confirmation of the mechanical reformat diff | `pnpm format && git diff --stat` shows the reformat; commit isolated from logic fixes for reviewability | +| CI step activates (was a no-op) | SC-2 | Requires a real PR to main on the Gitea runner | Open PR; observe `fast-checks` lint + format:check steps now report/gate instead of silently passing | + +--- + +## Validation Sign-Off + +- [ ] All tasks have a gate command (`pnpm lint` / `pnpm format:check`) or are Wave-1 config prerequisites +- [ ] Sampling continuity: lint runs after every task commit (gate is the unit of feedback) +- [ ] Deliberate-violation smoke test documented (SC-1) and executed once +- [ ] No watch-mode flags in CI +- [ ] Feedback latency < 60s +- [ ] `nyquist_compliant: true` set in frontmatter after planner maps task IDs + +**Approval:** pending -- 2.54.0 From 2863025bcd2af3dfab8113005c505a6d63498098 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 11 Jun 2026 19:18:46 -0400 Subject: [PATCH 05/24] docs(13): add pattern map --- .../13-real-lint-gate-eslint/13-PATTERNS.md | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 .planning/phases/13-real-lint-gate-eslint/13-PATTERNS.md diff --git a/.planning/phases/13-real-lint-gate-eslint/13-PATTERNS.md b/.planning/phases/13-real-lint-gate-eslint/13-PATTERNS.md new file mode 100644 index 0000000..6d16573 --- /dev/null +++ b/.planning/phases/13-real-lint-gate-eslint/13-PATTERNS.md @@ -0,0 +1,261 @@ +# Phase 13: Real Lint Gate (ESLint) - Pattern Map + +**Mapped:** 2026-06-11 +**Files analyzed:** 8 (3 new config files + 3 modified package.json/ci.yml + 2 source-fix files) +**Analogs found:** 5 / 8 (3 config files are greenfield with no in-repo analog) + +--- + +## File Classification + +| New/Modified File | Role | Data Flow | Closest Analog | Match Quality | +|---|---|---|---|---| +| `eslint.config.js` (root) | config | — | none | greenfield | +| `.prettierrc` (root) | config | — | none | greenfield | +| `.prettierignore` (root) | config | — | none | greenfield | +| `package.json` (root) | config | — | existing `lint`/`typecheck` scripts (lines 11-12) | exact | +| `apps/api/package.json` | config | — | existing `typecheck` script (line 13) | exact | +| `apps/pwa/package.json` | config | — | existing `typecheck` script (line 11) | exact | +| `.gitea/workflows/ci.yml` | config | — | existing `Lint` + `Typecheck` steps (lines 31-36) | exact | +| `apps/api/src/broker/*.ts` (violation fixes) | service | batch | existing setInterval+.catch pattern in same files | exact | +| `apps/pwa/src/components/EventForm.tsx` (violation fix) | component | request-response | existing `eslint-disable-next-line` + justification comment at line 274 | exact | + +--- + +## Pattern Assignments + +### `eslint.config.js` (root) — GREENFIELD + +No analog exists in the codebase. Use the full skeleton from RESEARCH.md Pattern 1 verbatim. Key constraints from the codebase: + +- Root `package.json` has `"type": "module"` absent (checked: it does NOT have `type: module`). However both `apps/api` and `apps/pwa` do (`"type": "module"`). The flat config file must be `eslint.config.js` (ESM); since root has no `type: module`, use `.mjs` extension OR add `"type": "module"` to root `package.json` alongside the new scripts. Prefer `eslint.config.js` + add `"type": "module"` to root — the root has no CJS code to break. +- `apps/api/tsconfig.json` excludes `"tests"` — add `apps/api/tests/**/*.ts` to the `disableTypeChecked` override block alongside the named config files. +- All three worker files (`outboxWorker.ts`, `poller.ts`, `reminderScheduler.ts`) use `setInterval(() => { runX().catch(...) }, N)` — the `.catch()` chain correctly handles the promise. If `no-floating-promises` fires on the `.catch()` return value itself, wrap with `void`: `void runX().catch(...)`. + +**Config files that need `disableTypeChecked` override** (confirmed outside all tsconfig `include` arrays): +- `apps/api/drizzle.config.ts` +- `apps/api/vitest.config.ts` +- `apps/pwa/vite.config.ts` +- `apps/pwa/vitest.config.ts` +- `apps/pwa/playwright.config.ts` +- `apps/api/tests/**/*.ts` (excluded from `apps/api/tsconfig.json`) + +--- + +### `.prettierrc` / `.prettierignore` — GREENFIELD + +No analog exists. Use RESEARCH.md Prettier Configuration section verbatim. + +Codebase style observation: all existing source files use single quotes (confirmed by broker file imports like `import { z } from 'zod'`). Set `"singleQuote": true`. + +--- + +### `package.json` (root) — adding `format` / `format:check` scripts + +**Analog:** the existing `lint` and `typecheck` script wiring in the same file. + +**Existing pattern** (`/home/luc/Projects/familysync/package.json`, lines 11-12): +```json +"lint": "pnpm -r --if-present lint", +"typecheck": "pnpm -r typecheck" +``` + +**Apply the same style** for new scripts: +```json +"format": "prettier --write .", +"format:check": "prettier --check ." +``` + +Note: root `package.json` currently has no `"type": "module"`. Adding it is required so `eslint.config.js` (ESM) is parsed as ESM by Node. Root has no `index.js` or other CJS entry points that would break. + +--- + +### `apps/api/package.json` — adding `lint` script + +**Analog:** the existing `typecheck` script in the same file (`/home/luc/Projects/familysync/apps/api/package.json`, line 13): +```json +"typecheck": "tsc --noEmit" +``` + +**Pattern to mirror** — same position in the scripts block, same style: +```json +"lint": "eslint src/ tests/ --max-warnings 0" +``` + +Mirror pattern: single command, no wrapper, scope by directory. The `tests/` directory is listed here even though it is excluded from the tsconfig — the ESLint config will apply `disableTypeChecked` for those files so the linter won't fail on project-inclusion errors. + +--- + +### `apps/pwa/package.json` — adding `lint` script + +**Analog:** the existing `typecheck` script in the same file (`/home/luc/Projects/familysync/apps/pwa/package.json`, line 11): +```json +"typecheck": "tsc --noEmit && tsc --project tsconfig.e2e.json --noEmit" +``` + +**Pattern to mirror:** +```json +"lint": "eslint src/ e2e/ --max-warnings 0" +``` + +The `e2e/` directory is covered by `tsconfig.e2e.json`, which `projectService: true` discovers automatically — no special override needed for e2e specs. + +--- + +### `.gitea/workflows/ci.yml` — adding `Format check` step + +**Analog:** the existing `Lint` and `Typecheck` steps in the `fast-checks` job (`/home/luc/Projects/familysync/.gitea/workflows/ci.yml`, lines 31-36): +```yaml + - name: Lint + run: pnpm lint + + - name: Typecheck + run: pnpm typecheck +``` + +**New step — insert between `Lint` and `Typecheck`** (per RESEARCH ordering: lint → format:check → typecheck → tests): +```yaml + - name: Lint + run: pnpm lint + + - name: Format check + run: pnpm format:check + + - name: Typecheck + run: pnpm typecheck +``` + +Also remove the now-stale comment above the `Lint` step (lines 28-30): +```yaml + # lint is currently a no-op: no package defines a `lint` script and ESLint is + # not installed. `pnpm -r lint` prints ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT but + # exits 0, so this step passes. Wiring lint is out of this phase's scope. +``` + +--- + +### `apps/api/src/broker/*.ts` — `no-floating-promises` / `no-unsafe-*` violation fixes + +**Analog:** the existing setInterval worker pattern in the same files. All three workers (`outboxWorker.ts`, `poller.ts`, `reminderScheduler.ts`) already use the same correct structure: + +**Existing correct pattern** (`outboxWorker.ts`, lines 781-787): +```ts +export function startOutboxWorker(): void { + setInterval(() => { + runOutboxDrain().catch((err: unknown) => { + console.error('[outboxWorker] Unhandled runOutboxDrain error:', err) + }) + }, 15 * 1000) +} +``` + +This pattern (inner `.catch()` inside `setInterval` arrow) is already correct for `no-floating-promises` — the promise is handled. However `no-floating-promises` may fire on the `.catch()` return value itself (the chained promise is also a Promise). If so, the fix is: + +```ts +setInterval(() => { + void runOutboxDrain().catch((err: unknown) => { + console.error('[outboxWorker] Unhandled runOutboxDrain error:', err) + }) +}, 15 * 1000) +``` + +The `void` operator here is legitimate: it signals intentional fire-and-forget within a `setInterval` that handles its own scheduling. This is NOT masking a bug. + +**For `no-unsafe-member-access` on ical.js in `sync.ts`** (`apps/api/src/broker/sync.ts`, lines 99-125): the existing pattern uses `as string | null` casts on `getFirstPropertyValue()` returns. If the rule fires, the correct response is a targeted disable WITH the existing comment pattern: + +```ts +// ical.js getFirstPropertyValue returns a union type; cast to ICAL.Time for date handling +// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment +const dtstart = vevent.getFirstPropertyValue('dtstart') as ICAL.Time | null +``` + +The file already has an inline comment at line 99 explaining the cast — the justification comment is already there. The `eslint-disable-next-line` line is the only addition required per D-13-06. + +--- + +### `apps/pwa/src/components/EventForm.tsx` — `no-explicit-any` violation fix + +**Analog:** the file already has an `eslint-disable-next-line` with justification comment at line 274 (the line immediately before the violation): + +**Existing pattern** (`EventForm.tsx`, lines 271-275): +```ts + // (the API expand contract does not expose it in v1 — D-03). We cast to any to + // read it if a future API version adds it, and default to 'none' when not present + // (WR-03 v1 comment: occurrence edits whose recurrence is not in the cache default + // to 'none'; this will be addressed when the occurrence/expand contract is extended). + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const derivedRecurrence = (occurrence as any)?.recurrence as RecurrencePreset | undefined +``` + +The `eslint-disable-next-line` comment is already present in the source. This file requires no change for the `no-explicit-any` rule — the suppression is pre-existing and already justified. Verify the comment is on the line immediately before the `const derivedRecurrence` line and that no other `as any` casts exist in the file. + +--- + +## Shared Patterns + +### `typecheck` → `lint` script mirroring + +**Source:** all three `package.json` files +**Apply to:** `apps/api/package.json`, `apps/pwa/package.json` + +The `typecheck` scripts are the canonical pattern to copy for new per-package tool scripts: +- One command per script +- No wrapper (`pnpm run` prefix, extra flags) +- Directory-scoped (not `--ext`, not glob patterns) + +### `eslint-disable-next-line` with justification comment + +**Source:** `apps/pwa/src/components/EventForm.tsx` lines 271-275 +**Apply to:** any suppression required in `apps/api/src/broker/sync.ts` or `apps/api/src/broker/expand.ts` + +Pattern: multi-line comment block explaining WHY the rule is wrong here, immediately followed by `// eslint-disable-next-line `, immediately followed by the flagged line. No blank lines between comment, disable, and code. + +### CI step insertion + +**Source:** `.gitea/workflows/ci.yml` lines 31-38 +**Apply to:** new `Format check` step + +Pattern: `- name: Verb noun` (title case, imperative verb), ` run: pnpm ` in title → `heading.innerHTML` does NOT contain `Team Meeting', description: 'Bold description', location: 'Room', -} +}; const ALLDAY_OCCURRENCE: CalendarOccurrence = { id: 'allday-uid::2026-06-20', @@ -90,182 +86,182 @@ const ALLDAY_OCCURRENCE: CalendarOccurrence = { location: null, description: null, hasRrule: false, -} +}; // ── Import component (after mocks are declared) ─────────────────────────────── -import { EventDetailPopover } from './EventDetailPopover.js' -import { useCalendarStore } from '../store/calendarStore.js' +import { EventDetailPopover } from './EventDetailPopover.js'; +import { useCalendarStore } from '../store/calendarStore.js'; // ── Helpers ─────────────────────────────────────────────────────────────────── function renderPopover(occurrence = TIMED_OCCURRENCE) { - mockOpenEventId = occurrence.id - ;(useCalendarStore as unknown as ReturnType).mockImplementation( + mockOpenEventId = occurrence.id; + (useCalendarStore as unknown as ReturnType).mockImplementation( (selector?: (s: Record) => unknown) => { const state = { openEventId: mockOpenEventId, setOpenEventId: mockSetOpenEventId, setEventForm: mockSetEventForm, setDeleteDialog: mockSetDeleteDialog, - } - if (typeof selector === 'function') return selector(state) - return state + }; + if (typeof selector === 'function') return selector(state); + return state; }, - ) + ); const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } }, - }) + }); // Pre-populate the events cache so EventDetailPopover can resolve by id - client.setQueryData(['events'], { occurrences: [occurrence] }) + client.setQueryData(['events'], { occurrences: [occurrence] }); return render( , - ) + ); } // ── Tests ───────────────────────────────────────────────────────────────────── describe('EventDetailPopover', () => { beforeEach(() => { - vi.clearAllMocks() - mockOpenEventId = null - }) + vi.clearAllMocks(); + mockOpenEventId = null; + }); it('renders the event title as a heading', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByRole('heading')).toHaveTextContent('Team Standup') - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByRole('heading')).toHaveTextContent('Team Standup'); + }); it('renders location when present', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByText(/Conference Room B/)).toBeDefined() - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByText(/Conference Room B/)).toBeDefined(); + }); it('renders description when present', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByText(/Daily team sync meeting/)).toBeDefined() - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByText(/Daily team sync meeting/)).toBeDefined(); + }); it('renders owner name in footer for personal events', () => { - renderPopover(TIMED_OCCURRENCE) + renderPopover(TIMED_OCCURRENCE); // TIMED_OCCURRENCE is personal (isShared:false) with ownerName:'Alice' - expect(screen.getByText(/Alice/)).toBeDefined() - }) + expect(screen.getByText(/Alice/)).toBeDefined(); + }); it('renders "Family" in footer for shared calendar events', () => { - renderPopover(ALLDAY_OCCURRENCE) + renderPopover(ALLDAY_OCCURRENCE); // ALLDAY_OCCURRENCE has isShared:true — footer must show 'Family' - expect(screen.getByText('Family')).toBeDefined() - }) + expect(screen.getByText('Family')).toBeDefined(); + }); it('renders calendarName in footer when ownerName is null', () => { const noOwnerName: CalendarOccurrence = { ...TIMED_OCCURRENCE, id: 'no-owner-uid::2026-06-15T10:00:00', ownerName: null, - } - renderPopover(noOwnerName) - expect(screen.getByText(/My Calendar/)).toBeDefined() - }) + }; + renderPopover(noOwnerName); + expect(screen.getByText(/My Calendar/)).toBeDefined(); + }); it('renders an all-day event without crashing', () => { - renderPopover(ALLDAY_OCCURRENCE) - expect(screen.getByRole('heading')).toHaveTextContent('Birthday Party') - }) + renderPopover(ALLDAY_OCCURRENCE); + expect(screen.getByRole('heading')).toHaveTextContent('Birthday Party'); + }); it('close button has aria-label="Close"', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByLabelText('Close')).toBeDefined() - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByLabelText('Close')).toBeDefined(); + }); it('pressing Escape calls setOpenEventId(null)', () => { - renderPopover(TIMED_OCCURRENCE) - fireEvent.keyDown(document, { key: 'Escape' }) - expect(mockSetOpenEventId).toHaveBeenCalledWith(null) - }) + renderPopover(TIMED_OCCURRENCE); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(mockSetOpenEventId).toHaveBeenCalledWith(null); + }); it('clicking the close button calls setOpenEventId(null)', () => { - renderPopover(TIMED_OCCURRENCE) - fireEvent.click(screen.getByLabelText('Close')) - expect(mockSetOpenEventId).toHaveBeenCalledWith(null) - }) + renderPopover(TIMED_OCCURRENCE); + fireEvent.click(screen.getByLabelText('Close')); + expect(mockSetOpenEventId).toHaveBeenCalledWith(null); + }); it('clicking the backdrop calls setOpenEventId(null)', () => { - renderPopover(TIMED_OCCURRENCE) - fireEvent.click(screen.getByTestId('popover-backdrop')) - expect(mockSetOpenEventId).toHaveBeenCalledWith(null) - }) + renderPopover(TIMED_OCCURRENCE); + fireEvent.click(screen.getByTestId('popover-backdrop')); + expect(mockSetOpenEventId).toHaveBeenCalledWith(null); + }); it('renders nothing when openEventId is null', () => { - mockOpenEventId = null - ;(useCalendarStore as unknown as ReturnType).mockImplementation( + mockOpenEventId = null; + (useCalendarStore as unknown as ReturnType).mockImplementation( (selector?: (s: Record) => unknown) => { const state = { openEventId: null, setOpenEventId: mockSetOpenEventId, setEventForm: mockSetEventForm, setDeleteDialog: mockSetDeleteDialog, - } - if (typeof selector === 'function') return selector(state) - return state + }; + if (typeof selector === 'function') return selector(state); + return state; }, - ) + ); const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } }, - }) + }); const { container } = render( , - ) - expect(container.firstChild).toBeNull() - }) + ); + expect(container.firstChild).toBeNull(); + }); it('XSS guard: HTML in title renders as escaped text, not as DOM elements', () => { - renderPopover(OCCURRENCE_WITH_HTML) - const heading = screen.getByRole('heading') + renderPopover(OCCURRENCE_WITH_HTML); + const heading = screen.getByRole('heading'); // Team Meeting') - }) + expect(heading.textContent).toContain('Team Meeting'); + }); it('XSS guard: HTML in description renders as escaped text', () => { - renderPopover(OCCURRENCE_WITH_HTML) - const descEl = screen.getByTestId('event-description') + renderPopover(OCCURRENCE_WITH_HTML); + const descEl = screen.getByTestId('event-description'); // must NOT be rendered as a bold element - expect(descEl.innerHTML).not.toContain('') - expect(descEl.textContent).toContain('Bold description') - }) + expect(descEl.innerHTML).not.toContain(''); + expect(descEl.textContent).toContain('Bold description'); + }); // ── Phase 3 footer: Edit/Delete actions ──────────────────────────────────── it('footer renders an "Edit" button', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByRole('button', { name: /edit/i })).toBeInTheDocument() - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByRole('button', { name: /edit/i })).toBeInTheDocument(); + }); it('footer renders a "Delete" button', () => { - renderPopover(TIMED_OCCURRENCE) - expect(screen.getByRole('button', { name: /delete/i })).toBeInTheDocument() - }) + renderPopover(TIMED_OCCURRENCE); + expect(screen.getByRole('button', { name: /delete/i })).toBeInTheDocument(); + }); it('clicking "Edit" opens EventForm in edit mode and closes popover', () => { - renderPopover(TIMED_OCCURRENCE) - fireEvent.click(screen.getByRole('button', { name: /edit/i })) - expect(mockSetEventForm).toHaveBeenCalledWith(true, 'edit', TIMED_OCCURRENCE.uid) - expect(mockSetOpenEventId).toHaveBeenCalledWith(null) - }) + renderPopover(TIMED_OCCURRENCE); + fireEvent.click(screen.getByRole('button', { name: /edit/i })); + expect(mockSetEventForm).toHaveBeenCalledWith(true, 'edit', TIMED_OCCURRENCE.uid); + expect(mockSetOpenEventId).toHaveBeenCalledWith(null); + }); it('clicking "Delete" opens DeleteConfirmationDialog (setDeleteDialog)', () => { - renderPopover(TIMED_OCCURRENCE) - fireEvent.click(screen.getByRole('button', { name: /delete/i })) - expect(mockSetDeleteDialog).toHaveBeenCalledWith(true, TIMED_OCCURRENCE.uid) - }) + renderPopover(TIMED_OCCURRENCE); + fireEvent.click(screen.getByRole('button', { name: /delete/i })); + expect(mockSetDeleteDialog).toHaveBeenCalledWith(true, TIMED_OCCURRENCE.uid); + }); it('BUG-3 regression: IANA-bracketed start/end does not produce "Invalid Date" in rendered output', () => { // Fastmail events are serialized with IANA bracket notation e.g. '2026-06-18T08:00:00-04:00[America/Toronto]'. @@ -277,16 +273,16 @@ describe('EventDetailPopover', () => { uid: 'iana-bracket-uid', start: '2026-06-18T08:00:00-04:00[America/Toronto]', end: '2026-06-18T09:00:00-04:00[America/Toronto]', - } - renderPopover(occurrence) + }; + renderPopover(occurrence); // The date/time text must not contain 'Invalid Date' - const dialogEl = screen.getByRole('dialog') - expect(dialogEl.textContent).not.toContain('Invalid Date') + const dialogEl = screen.getByRole('dialog'); + expect(dialogEl.textContent).not.toContain('Invalid Date'); // It must contain recognizable date content (month name or a digit) // toLocaleDateString output varies by locale; check for a digit at minimum - const dateTimeText = dialogEl.textContent ?? '' - expect(dateTimeText).toMatch(/\d/) - }) -}) + const dateTimeText = dialogEl.textContent ?? ''; + expect(dateTimeText).toMatch(/\d/); + }); +}); diff --git a/apps/pwa/src/components/EventDetailPopover.tsx b/apps/pwa/src/components/EventDetailPopover.tsx index a57e7b7..26d144a 100644 --- a/apps/pwa/src/components/EventDetailPopover.tsx +++ b/apps/pwa/src/components/EventDetailPopover.tsx @@ -22,26 +22,26 @@ * - aria-modal="true", role="dialog" */ -import { useEffect, useRef } from 'react' -import { useQueryClient } from '@tanstack/react-query' -import { MapPin, Edit2, Trash2 } from 'lucide-react' -import { useCalendarStore } from '../store/calendarStore.js' -import type { CalendarOccurrence } from '../api/client.js' +import { useEffect, useRef } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; +import { MapPin, Edit2, Trash2 } from 'lucide-react'; +import { useCalendarStore } from '../store/calendarStore.js'; +import type { CalendarOccurrence } from '../api/client.js'; // ── Types ────────────────────────────────────────────────────────────────── /** Shape of props passed by Schedule-X customComponents.eventModal */ interface ScheduleXEventModalProps { calendarEvent?: { - id?: string | number - title?: string - start?: unknown - end?: unknown - calendarId?: string - location?: string - description?: string - _familySync?: { uid: string; color: string; isShared: boolean } - } + id?: string | number; + title?: string; + start?: unknown; + end?: unknown; + calendarId?: string; + location?: string; + description?: string; + _familySync?: { uid: string; color: string; isShared: boolean }; + }; } // ── Helpers ──────────────────────────────────────────────────────────────── @@ -58,41 +58,41 @@ function formatDateTime(start: string, end: string, allDay: boolean): string { if (allDay) { // YYYY-MM-DD — format as a date without time try { - const d = new Date(start + 'T00:00:00') + const d = new Date(start + 'T00:00:00'); return d.toLocaleDateString(undefined, { weekday: 'short', year: 'numeric', month: 'long', day: 'numeric', - }) + }); } catch { - return start + return start; } } // Timed — parse offset-aware ISO string. // Strip trailing IANA bracket e.g. '[America/Toronto]' before passing to new Date(): // new Date() cannot parse the bracket notation and returns Invalid Date. try { - const cleanStart = start.replace(/\[[^\]]*\]$/, '') - const cleanEnd = end.replace(/\[[^\]]*\]$/, '') - const startDate = new Date(cleanStart) - const endDate = new Date(cleanEnd) + const cleanStart = start.replace(/\[[^\]]*\]$/, ''); + const cleanEnd = end.replace(/\[[^\]]*\]$/, ''); + const startDate = new Date(cleanStart); + const endDate = new Date(cleanEnd); const dateStr = startDate.toLocaleDateString(undefined, { weekday: 'short', month: 'long', day: 'numeric', - }) + }); const startTime = startDate.toLocaleTimeString(undefined, { hour: 'numeric', minute: '2-digit', - }) + }); const endTime = endDate.toLocaleTimeString(undefined, { hour: 'numeric', minute: '2-digit', - }) - return `${dateStr}, ${startTime} – ${endTime}` + }); + return `${dateStr}, ${startTime} – ${endTime}`; } catch { - return start + return start; } } @@ -106,65 +106,65 @@ function formatDateTime(start: string, end: string, allDay: boolean): string { * In standalone mode it resolves the event from TanStack Query cache. */ export function EventDetailPopover(props: ScheduleXEventModalProps = {}) { - const { openEventId, setOpenEventId } = useCalendarStore() - const setEventForm = useCalendarStore((s) => s.setEventForm) - const setDeleteDialog = useCalendarStore((s) => s.setDeleteDialog) - const queryClient = useQueryClient() - const dialogRef = useRef(null) + const { openEventId, setOpenEventId } = useCalendarStore(); + const setEventForm = useCalendarStore((s) => s.setEventForm); + const setDeleteDialog = useCalendarStore((s) => s.setDeleteDialog); + const queryClient = useQueryClient(); + const dialogRef = useRef(null); // Resolve the event to display: // 1. If Schedule-X passed a calendarEvent prop, use it to get the id // 2. Otherwise use Zustand openEventId const activeId: string | null = (() => { if (props.calendarEvent?.id != null) { - return String(props.calendarEvent.id) + return String(props.calendarEvent.id); } - return openEventId - })() + return openEventId; + })(); // Lookup the occurrence in TanStack Query cache. // We search all 'events' query entries for a matching id. const occurrence: CalendarOccurrence | null = (() => { - if (!activeId) return null + if (!activeId) return null; // queryClient.getQueriesData returns [{queryKey, data}] entries const allEntries = queryClient.getQueriesData<{ occurrences: CalendarOccurrence[] }>({ queryKey: ['events'], - }) + }); for (const [, data] of allEntries) { - if (!data?.occurrences) continue - const found = data.occurrences.find((o) => o.id === activeId) - if (found) return found + if (!data?.occurrences) continue; + const found = data.occurrences.find((o) => o.id === activeId); + if (found) return found; } - return null - })() + return null; + })(); // Close handler - const handleClose = () => setOpenEventId(null) + const handleClose = () => setOpenEventId(null); // Escape key listener — add to document so it works even when focus is trapped useEffect(() => { - if (!activeId) return + if (!activeId) return; const onKeyDown = (e: KeyboardEvent) => { if (e.key === 'Escape') { - handleClose() + handleClose(); } - } - document.addEventListener('keydown', onKeyDown) - return () => document.removeEventListener('keydown', onKeyDown) - }, [activeId]) // eslint-disable-line react-hooks/exhaustive-deps + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, [activeId]); // eslint-disable-line react-hooks/exhaustive-deps // Focus trap — when popover opens, focus the dialog useEffect(() => { if (activeId && dialogRef.current) { - dialogRef.current.focus() + dialogRef.current.focus(); } - }, [activeId]) + }, [activeId]); // Nothing to show - if (!activeId || !occurrence) return null + if (!activeId || !occurrence) return null; // Responsive: detect phone breakpoint - const isPhone = typeof window !== 'undefined' && window.matchMedia('(max-width: 767px)').matches + const isPhone = typeof window !== 'undefined' && window.matchMedia('(max-width: 767px)').matches; const dialogStyle: React.CSSProperties = isPhone ? { @@ -198,7 +198,7 @@ export function EventDetailPopover(props: ScheduleXEventModalProps = {}) { overflowY: 'auto', zIndex: 200, fontFamily: 'var(--font-family-base)', - } + }; return ( <> @@ -316,11 +316,7 @@ export function EventDetailPopover(props: ScheduleXEventModalProps = {}) { fontFamily: 'var(--font-family-base)', }} > -