release: promote beta to main

.claude/agent-memory/e2e-test-engineer/MEMORY.md

@@ -1,7 +1,26 @@
 # E2E Test Engineer — Agent Memory (Index)
 
 > Detailed notes live in topic files. This index links to them.
-> See: `e2e-pom-patterns.md`, `e2e-parallel-isolation.md`, `story-epic08-e2e.md`, `story-933-dav-vendor-contacts.md`
+> See: `e2e-pom-patterns.md`, `e2e-parallel-isolation.md`, `story-epic08-e2e.md`, `story-933-dav-vendor-contacts.md`, `milestones-e2e.md`
+
+## Invoices + Manage Settings E2E (2026-03-26) — Fixed 2026-03-26
+
+POMs: `InvoicesPage.ts`, `InvoiceDetailPage.ts`, `HouseholdItemEditPage.ts`.
+Tests: `e2e/tests/invoices/invoices.spec.ts`, `e2e/tests/navigation/settings-manage.spec.ts`.
+Key API response shapes: Areas POST → `{area:{id}}`, Trades POST → `{trade:{id}}`, HI-Categories POST → `{id}` (entity directly), Invoices POST → `{invoice:{id}}`.
+InvoicesPage.heading = "Budget" (from PageLayout title=t('invoices.title')). Modal locator: `getByRole('dialog',{name:/Invoice/i})`.
+InvoiceDetailPage: edit modal `[role="dialog"][aria-labelledby="edit-modal-title"]`, delete modal `[aria-labelledby="delete-modal-title"]`, confirm delete button `[class*="confirmDeleteButton"]`.
+ManagePage tab panel IDs: `areas-panel`, `trades-panel`, `budget-categories-panel`, `hi-categories-panel`. Create form IDs: `#areaName`, `#tradeName`, `#categoryName` (same for budget AND hi-cat tabs — only one renders at a time).
+**ManagePage area/trade delete buttons have NO aria-label** — only text "Delete". Must scope via
+`panel.locator('[class*="itemRow"]').filter({ hasText: entityName }).getByRole('button', { name: 'Delete', exact: true })`.
+HI-categories delete buttons DO have `aria-label={Delete \${name}}` — getByRole with name works.
+InvoicesPage.waitForLoaded() uses Promise.any() (not Promise.race()) to avoid dangling rejections.
+
+## Milestones E2E (2026-03-26) — See milestones-e2e.md
+
+Heading="Project", newMilestone=testId("new-milestone-button"), search=client-side (no waitForResponse).
+List deleteModal=`getByRole('dialog',{name:'Delete Milestone'})`. Detail deleteModal=`[role="dialog"][aria-modal="true"]` (own impl).
+Milestone IDs are integers (not strings). Back/cancel on CreatePage are `<Link>` anchors, not buttons.
 
 ## i18n German Locale: page.reload() Required After setLanguage() + page.goto() (2026-03-23)
 

.claude/agent-memory/e2e-test-engineer/milestones-e2e.md

@@ -0,0 +1,74 @@
+---
+name: milestones-e2e
+description: Key POM selectors and patterns for the milestones E2E tests written in 2026-03
+type: project
+---
+
+# Milestones E2E — Selectors and Patterns
+
+**Files**: `e2e/pages/MilestonesPage.ts`, `e2e/pages/MilestoneCreatePage.ts`, `e2e/pages/MilestoneDetailPage.ts`, `e2e/tests/milestones/milestones.spec.ts`
+
+## Key Selectors
+
+### MilestonesPage (list)
+
+- Heading: `getByRole('heading', { level: 1, name: 'Project' })` — PageLayout h1 = "Project" (milestones.page.title)
+- New Milestone: `getByTestId('new-milestone-button')` — stable data-testid on the button
+- Search: `getByLabel('Search items')` — client-side filtering, NO waitForResponse needed after fill()
+- Delete modal: `getByRole('dialog', { name: 'Delete Milestone' })` — shared Modal component
+- Confirm delete: `[class*="btnConfirmDelete"]` inside delete modal
+- Actions menu: `[aria-label="Actions menu"]` in table row/card
+
+### MilestoneCreatePage
+
+- Page h1: `getByRole('heading', { level: 1, name: 'Project' })` — same as list (PageLayout)
+- Form h2: `getByRole('heading', { level: 2, name: 'Create Milestone' })`
+- Back link: `getByRole('link', { name: '← Milestones' })` — it's an <a>, NOT <button>
+- Cancel: `getByRole('link', { name: 'Cancel' })` — also a <Link> (<a>), NOT <button>
+- Title: `locator('#title')` or `getByTestId('milestone-title-input')`
+- Target date: `locator('#targetDate')` or `getByTestId('milestone-target-date-input')`
+- Submit: `getByTestId('create-milestone-button')`
+- Error banner: `locator('[role="alert"][class*="errorBanner"]')` — used for BOTH validation + server errors
+
+### MilestoneDetailPage
+
+- h1: `getByRole('heading', { level: 1 })` — the milestone title (dynamic)
+- Back button: `getByRole('button', { name: /← Back to Milestones/i })` — <button> NOT <a>
+- Edit button: `getByTestId('edit-milestone-button')`
+- Delete button: `getByTestId('delete-milestone-button')`
+- Status badge: `locator('[class*="statusBadge"]')` — text "Completed" or "Pending"
+- Save button: `getByTestId('save-milestone-button')`
+- Completed checkbox: `getByTestId('milestone-completed-checkbox')`
+- Delete modal: `locator('[role="dialog"][aria-modal="true"]')` — own implementation (NOT shared Modal)
+- Confirm delete: `getByTestId('confirm-delete-milestone')`
+- Not found state: `locator('[class*="notFound"]')`
+
+## Critical Behavioral Notes
+
+1. **MilestonesPage search is client-side**: All milestones are loaded once at mount. The DataTable
+   filters them synchronously on search input change. No API waitForResponse after fill().
+   However, `waitForLoaded()` after `goto()` is still needed to wait for the initial GET.
+
+2. **List page heading = "Project"**: Both MilestonesPage and WorkItemsPage use PageLayout with
+   `title={t('milestones.page.title')}` = "Project" — the SubNav distinguishes which tab is active.
+
+3. **getMilestoneTitles() fallback**: No `[class*="itemLink"]` or `[class*="vendorLink"]` in
+   milestones — the title column renders plain text. Read first `td` of each row (desktop) or
+   first `cardCell` (mobile).
+
+4. **Detail page uses its own modal, not shared Modal**: The MilestoneDetailPage delete modal is
+   a custom `<div role="dialog" aria-modal="true">`. The MilestonesPage list delete modal uses
+   the shared `<Modal>` component. Selectors differ between the two pages.
+
+5. **createMilestoneViaApi returns number**: Unlike work items (string UUID), milestone IDs are
+   integers. Use `createdId: number | null` in tests.
+
+6. **POST /api/milestones response shape — NO wrapper**: The server returns `MilestoneDetail`
+   directly (e.g. `{ id: 1, title: "...", ... }`), NOT wrapped in `{ milestone: { id: 1 } }`.
+   This was a bug in the original test/helper code — `body.milestone.id` caused
+   `TypeError: Cannot read properties of undefined (reading 'id')` in CI. Correct pattern:
+   `const body = (await response.json()) as { id: number }; return body.id;`
+   Same applies to in-test POST response parsing in Scenarios 4 and 5.
+
+**Why**: Milestones feature had zero E2E coverage. Written as part of Gap-1 E2E work (2026-03).
+**How to apply**: When adding more milestone-related tests, reuse these POMs and patterns.

.claude/agent-memory/qa-integration-tester/MEMORY.md

@@ -3,6 +3,125 @@
 > Detailed notes live in topic files. This index links to them.
 > See: `budget-categories-story-142.md`, `e2e-pom-patterns.md`, `e2e-parallel-isolation.md`, `story-358-document-linking.md`, `story-360-document-a11y.md`, `story-epic08-e2e.md`, `story-509-manage-page.md`, `story-471-dashboard.md`
 
+## Fastify AJV Default: removeAdditional=true (2026-03-26)
+
+**Critical pattern**: Fastify's `@fastify/ajv-compiler` defaults to `removeAdditional: true`. This means `additionalProperties: false` in body/querystring schemas does NOT reject unknown properties with 400 — it strips them and lets the request proceed. Tests that expect 400 for unknown fields are wrong. The correct test is to assert the request succeeds (201/200) with extra fields silently removed. See `server/src/routes/auth.test.ts` comment for reference.
+
+**Affected test files fixed (2026-03-26)**: `invoiceBudgetLines.test.ts` (POST + PATCH), `standaloneInvoices.test.ts` (GET querystring).
+
+**Correct test pattern** (from `invoices.test.ts`):
+
+```ts
+it('strips unknown properties from request body (additionalProperties: false)', async () => {
+  // ...send with unknownField...
+  expect(response.statusCode).toBe(201); // Fastify strips extra props — still succeeds
+});
+```
+
+## Gap 5 — Client Vendor/Trade/Area Utility Tests (2026-03-26)
+
+**Files** (10 new): `client/src/lib/areasApi.test.ts`, `tradesApi.test.ts`, `vendorContactsApi.test.ts`, `davTokensApi.test.ts`, `timelineApi.test.ts`, `areaTreeUtils.test.ts`, `client/src/hooks/useAreas.test.ts`, `useTrades.test.ts`, `useVendorContacts.test.ts`, `useDavToken.test.ts`.
+
+**Key patterns**:
+
+- **`makeArea` helper**: Use function body + `{ ...defaults, ...overrides }` — NOT inline object literal with explicit + spread of same key. TypeScript TS2783 fires when `id`/`name` appear both explicitly and in `...overrides`.
+- **API tests for DELETE**: Use `status: 204, text: async () => ''` not `json: async () => undefined`. apiClient short-circuits at 204 and never calls `.json()`.
+- **`fetchAreas`/`fetchTrades` empty search**: Passing `{ search: '' }` should NOT include `?search=` param (falsy guard in source).
+- **`useVendorContacts` empty vendorId**: When vendorId is empty string, hook skips fetch immediately — assert `not.toHaveBeenCalled()` for listVendorContacts.
+- **`useDavToken` generate flow**: After `generateDavToken()`, hook calls `getDavTokenStatus()` again. Mock `getDavTokenStatus` must return success twice (initial mount + after generate).
+- **Pre-existing TS errors in worktree**: `usePhotos.test.ts`, `HouseholdItemsPage.test.tsx`, `VendorsPage.test.tsx`, `UserManagementPage.test.tsx` had TypeScript errors from other agents' work BEFORE this session. These are not caused by Gap 5 files.
+
+## Gap 3 (Client) — usePhotos hook + photoApi client Tests (2026-03-26)
+
+**Files**: `client/src/hooks/usePhotos.test.ts` (40+ tests), `client/src/lib/photoApi.test.ts` (30+ tests).
+
+**Key patterns**:
+
+- **`mockUploadPhotoApi` needs variadic type**: Hook mock typed as `jest.fn<() => Promise<unknown>>()` fails when `mockImplementationOnce` passes a 5-arg function. Fix: `jest.fn<(...args: any[]) => Promise<unknown>>()`.
+- **`mockXhr.open` / `mockXhr.send` must match call signatures**: If typed as `() => void`, `toHaveBeenCalledWith('POST', '/api/photos')` fails TypeScript (expects 0 args, called with 2). Fix: type as `(method: string, url: string) => void` and `(body?: FormData) => void`.
+- **FormData access**: With typed `send: MockedFunction<(body?: FormData) => void>`, use `mockXhr.send.mock.calls[0][0] as FormData` — no extra cast needed.
+- **XHR mock pattern**: Build `mockXhr` object inside `beforeEach`, override `globalThis.XMLHttpRequest = jest.fn(() => mockXhr) as unknown as typeof XMLHttpRequest`. Capture event handlers in closure vars (`xhrEventHandlers`, `xhrUploadEventHandlers`). Fire `xhrEventHandlers['load']()` to trigger promise resolution.
+- **`upload.addEventListener` should NOT be called when `onProgress` is not provided**: The hook checks `if (onProgress)` before registering. Assert `mockXhr.upload.addEventListener.not.toHaveBeenCalled()`.
+- **Clearing upload progress**: After success/failure, progress map entry is deleted. Test by capturing the internal `progressWrapper` via `mockImplementationOnce`, calling it, then awaiting the upload and asserting `uploadProgress.has(filename) === false`.
+
+## Gap 3 — photoService + photos route Tests (2026-03-26)
+
+**Files**: `server/src/services/photoService.test.ts` (51 tests), `server/src/routes/photos.test.ts` (45 tests).
+
+**Key patterns**:
+
+- **Mock `sharp` with `jest.unstable_mockModule('sharp', () => ({ default: mockSharpFn }))`**: sharp is not compiled for the test environment (native binary). Return a chainable mock instance.
+- **`AnyMock` type**: `type AnyMock = jest.MockedFunction<(...args: any[]) => any>` — use this for all mock functions where TypeScript infers `never` from bare `jest.fn()`. Cast with `jest.fn() as AnyMock`.
+- **Chainable sharp mock**: `mockSharpInstance` object with each method typed `as AnyMock` and each chain method returning `mockSharpInstance`. Use `mockResolvedValue` (not `Once`) in `beforeEach` to avoid state leakage between tests.
+- **FK constraint for `photos.createdBy`**: Cannot pass non-existent userId — SQLite throws FK error. Test `createdBy: null` by: upload with real user, delete user (cascade sets NULL), then `getPhoto`.
+- **`route` tests mock ALL service exports**: `deletePhotosForEntity` must be in the mock even if not directly used by route handlers. ESM named export binding validation fails otherwise.
+- **Auth guard lines unreachable via inject()**: `if (!request.user) throw new UnauthorizedError()` inside route handlers is never reached when global auth hook fires first. This is a known coverage gap (~8% branch coverage).
+- **Multipart body builder**: Build raw `multipart/form-data` bodies using `Buffer.concat()` with CRLF boundaries. Use `buildMultipartBody()` helper pattern.
+- **`readdirSync` import**: ESM has no `require()`. Always import `readdirSync` from `'node:fs'` at top of file.
+
+## Gap 7 — calendarIcal + vendorVcard Unit Tests (2026-03-26)
+
+**Files**: `server/src/services/calendarIcal.test.ts` (49 tests), `server/src/services/vendorVcard.test.ts` (46 tests). Both files were in commit `ba297480` on branch `chore/1204-test-coverage-gaps`.
+
+**Key patterns**:
+
+- **`calendarIcal.ts` exports**: `toDateOnly`, `computeETag`, `computeCalendarETag` (needs DB), `buildCalendar`, `DescriptionMap` type
+- **`vendorVcard.ts` exports**: `computeAddressBookETag` (needs DB), `buildVendorVcard`, `buildContactVcard`
+- **DB for ETag tests**: Use `Object.assign(drizzle(sqliteDb, {schema}), { $client: sqliteDb }) as DbType` pattern for in-memory SQLite
+- **`HouseholdItemCategory = string`** (type alias, not interface) — use `'Furniture'` not `{ id, name }`
+- **`WorkItemStatus`**: `'not_started' | 'in_progress' | 'completed'` (not `'planned'`)
+- **DescriptionMap key format**: `wi-{id}` for work items, `milestone-{id}` for milestones, `hi-{id}` for household items — matches what `buildCalendar` uses
+- **`buildVendorVcard` injects `KIND:org\r\nUID:...\r\nREV:...`** before `END:VCARD` via string replace
+- **`buildContactVcard` injects `UID:...\r\nREV:...`** before `END:VCARD` (no KIND:org)
+- **Multi-agent branch**: When multiple agents push to the same branch, CI failures may be from OTHER agents' commits, not yours. Check commit SHA in CI run to identify which commit caused failures.
+
+## Gap 2 — Invoice Budget Lines + Standalone Invoices Tests (2026-03-25)
+
+**Test files**: `invoiceBudgetLineService.test.ts` (service unit, ~40 tests), `invoiceBudgetLines.test.ts` (route integration, ~30 tests), `standaloneInvoices.test.ts` (route integration, ~30 tests).
+
+**Key patterns**:
+
+- **`invoiceBudgetLines` UNIQUE constraints**: `work_item_budget_id` and `household_item_budget_id` each have a partial unique index `WHERE NOT NULL`. This means each budget line can only link to ONE invoice. Test that linking same WIB to a 2nd invoice throws `BudgetLineAlreadyLinkedError` (409). Linking the same WIB to the SAME invoice throws `ValidationError` (400).
+- **`ItemizedSumExceedsInvoiceError` is 400**: Uses `super('ITEMIZED_SUM_EXCEEDS_INVOICE', 400, ...)`. Check body `error.code === 'ITEMIZED_SUM_EXCEEDS_INVOICE'`.
+- **`BudgetLineAlreadyLinkedError` is 409**: Uses `super('BUDGET_LINE_ALREADY_LINKED', 409, ...)`. Check body `error.code === 'BUDGET_LINE_ALREADY_LINKED'`.
+- **XOR validation**: Service checks `hasWorkItem` (non-null + defined) vs `hasHouseholdItem`. Providing `null` for one and a real ID for the other = valid one-sided link.
+- **`updateInvoiceBudgetLine` rejects budget ID changes**: Any attempt to set `workItemBudgetId` or `householdItemBudgetId` in update body → `ValidationError`, even if null.
+- **`householdItems` requires `categoryId: 'hic-furniture'`**: Seeded in migration; always use this. NOT NULL FK constraint.
+- **`listAllInvoices` returns `{ invoices, pagination, summary, filterMeta }`**: `summary` is GLOBAL (unfiltered); `filterMeta.amount` reflects base conditions (excluding amount range filter).
+- **`getInvoiceById` wraps result in `{ invoice }` envelope**: Route sends `{ invoice }` not bare object.
+- **Standalone invoice routes registered at `/api/invoices`**: Uses `standaloneInvoiceRoutes` prefix. Route for `/:invoiceId` conflicts with vendor subroutes at `/api/vendors/:vendorId/invoices/:invoiceId`.
+- **`additionalProperties: false` on `standaloneInvoices` querystring**: Unknown query params return 400.
+- **Sort options**: `sortBy` accepts `date|amount|status|vendor_name|due_date`. Invalid value → 400.
+- **`getInvoiceBudgetLinesForInvoice` uses raw SQL**: Uses `db.all<{...}>(sql\`...\`)`pattern for efficiency — test via`createInvoiceBudgetLine`first then verify`getInvoiceBudgetLinesForInvoice`result has correct`budgetLineId`, `budgetLineType`, `itemName`.
+
+## Gap 4+6 Client Page + API Tests (2026-03-25)
+
+**Files**: `VendorsPage.test.tsx`, `UserManagementPage.test.tsx`, `HouseholdItemsPage.test.tsx`, `workItemBudgetsApi.test.ts`, `workItemMilestonesApi.test.ts`.
+
+**Key patterns**:
+
+- **`WorkItemBudgetLine` fixture**: Uses `BaseBudgetLine` shape: `confidence: 'own_estimate'` (NOT `'medium'`), `confidenceMargin`, `budgetCategory/budgetSource/vendor: null`, `actualCost`, `actualCostPaid`, `invoiceLink: null`, `quantity/unit/unitPrice/includesVat: null`.
+- **`WorkItemMilestones` fixture**: `required/linked` arrays of `MilestoneSummaryForWorkItem` — only `{ id: number, name: string, targetDate: string | null }`. NOT `title`, `dueDate`, etc.
+- **`CreateBudgetLineRequest`**: `budgetSourceId`, `budgetCategoryId`, `plannedAmount`, `description` — NOT `sourceId/categoryId/notes`.
+- **Page tests need `useTableState` mock**: Pages using `useTableState` hook require mocking `../../hooks/useTableState.js` returning full object with `tableState`, `searchInput`, `setSearch`, `toApiParams`, `setFilter`.
+- **`HouseholdItemsPage` needs 4 mocks**: `householdItemsApi`, `vendorsApi`, `householdItemCategoriesApi`, `useAreas`.
+- **`UserManagementPage` needs `AuthContext` mock**: Imports `useAuth()` for `isAdmin` — mock `../../contexts/AuthContext.js`.
+- **`VendorsPage` needs `useTrades` + `TradePicker` mocks**: Both must be mocked to avoid cascading fetch calls in hook.
+- **Cannot run tests locally**: worktree node_modules/.bin is sparse (no jest binary). Commit and rely on CI.
+
+## Bug #1201 Backup Execution Path Tests (2026-03-25)
+
+**Test files modified**: `backupService.test.ts`, `backups.test.ts`.
+
+**Key patterns**:
+
+- **Real DB construction**: `import Database from 'better-sqlite3'; import { drizzle } from 'drizzle-orm/better-sqlite3'; const rawDb = new Database(join(tempDir, 'test.db')); const db = drizzle(rawDb);`. Close in `afterEach` with `if (rawDb && rawDb.open) rawDb.close()`.
+- **Mock `db.$client.backup`**: The service calls `getClient(db).backup(path)`. Mock with `{ $client: { backup: jest.fn().mockRejectedValue(...) } } as any`. Pass `Object.assign(new Error('...'), { code: 'SQLITE_IOERR' })` to simulate SqliteError.
+- **chmod read-only test requires non-root guard**: `if (process.getuid?.() === 0) { return; }` — chmod doesn't restrict root in CI containers. Restore permissions in `finally`/`afterEach` before directory cleanup.
+- **Two separate `mkdtempSync` calls for service tests**: Same pattern as route tests — `tempDir` for app data (DB path), `backupTempDir` for backups (must be outside app data dir per config validation).
+- **Retention test seeding**: Pre-write stub `.tar.gz` files with valid names (e.g. `cornerstone-backup-2026-01-01T000000Z.tar.gz`) using `writeFileSync`. After the real backup runs, assert `listBackups()` length equals retention limit and oldest stub is absent.
+- **Docker failure separate from tests**: All 6 test shards passed. Docker build failed because `COPY --from=deps /usr/bin/tar /usr/bin/tar` requires `tar` to be explicitly installed in the `deps` stage first (`apk add --no-cache tar`). This is a Dockerfile production fix, not a test issue.
+
 ## Story #1146 Backup/Restore Tests (2026-03-22)
 
 **Test files**: `backupService.test.ts`, `backups.test.ts` (routes), `backupsApi.test.ts`, `BackupsPage.test.tsx`.