steilerDev · steilerDev · Jul 8, 2026 · Jul 8, 2026
diff --git a/.claude/agent-memory/e2e-test-engineer/MEMORY.md b/.claude/agent-memory/e2e-test-engineer/MEMORY.md
@@ -19,8 +19,9 @@
 - [story-933-dav-vendor-contacts.md](story-933-dav-vendor-contacts.md) — vendor contacts / CardDAV story notes.
 - [story-1248-mass-move.md](story-1248-mass-move.md) — mass-move budget line story notes.
 - [story-epic08-e2e.md](story-epic08-e2e.md) — EPIC-08 Paperless integration (no testcontainer yet — all document tests validate "not configured" state only).
+- [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md) — root cause + fix for the shard-3 diary flakes blocking main promotions; also proves `maxFailures` already tolerates one retry-passing test (Playwright source citation).
 
 ## Open follow-ups to flag to orchestrator
 
-- Diary shard-3 E2E Gates flakiness recurred with NEW failure signatures as of 2026-07-07 despite fix PRs #1790/#1792/#1793 — see known-flakes-and-regressions.md. Needs dedicated investigation before more diary work.
 - No Paperless-ngx testcontainer exists yet (story-epic08-e2e.md) — all Paperless E2E coverage is `page.route()` mocked, not real integration. Add the container when Paperless work resumes.
+- Full containerized E2E verification is not possible in sandboxes without `dhi.io` (Docker Hardened Images) registry credentials — building `cornerstone:e2e` fails with `401 Unauthorized`. This is an environment limitation, not a code issue; verification in such sandboxes must fall back to static checks (lint/prettier/tsc-diff/`playwright --list`) plus post-merge CI observation.
diff --git a/.claude/agent-memory/e2e-test-engineer/bug-1829-shard3-flakes.md b/.claude/agent-memory/e2e-test-engineer/bug-1829-shard3-flakes.md
@@ -0,0 +1,125 @@
+---
+name: bug-1829-shard3-flakes
+description: Root cause + fix for the recurring E2E shard-3 diary flakes blocking main promotions (issue #1829) — confirms maxFailures/retry accounting semantics.
+metadata:
+  type: project
+---
+
+## Summary
+
+Issue #1829 reported 3 "failing" tests on shard 3/16 desktop on main-targeted PRs
+(#1791, #1828). Investigation (job 85332065279 logs, `gh api repos/.../actions/jobs/<id>/logs`)
+showed only **one** was a real, independent bug — the other two were consequences of
+`maxFailures: 1` fail-fast, not independent defects:
+
+1. **`e2e/tests/diary/diary-drafts.spec.ts:854`** (Scenario 14) — REAL bug. Failed on
+   BOTH attempt 1 and retry #1 (48s each) → genuinely exhausted retries → correctly
+   tripped `maxFailures: 1`.
+2. **`e2e/tests/documents/document-linking.spec.ts:369`** (Scenario 7a) — NOT a real
+   bug. Log shows it ran only 1.6s before being marked `✘` — it was cancelled
+   mid-flight as collateral damage the instant #1 tripped fail-fast, not a genuine
+   assertion/timeout failure of its own.
+3. **`e2e/tests/diary/diary-r2-uat.spec.ts:599`** (Scenario 10) — REAL secondary bug
+   (a genuine race), but reported as "flaky" (failed attempt 1, passed retry) — this
+   did NOT trip `maxFailures` (see accounting proof below), so it did not cause the
+   shard cancellation; it's independently worth fixing but wasn't the trigger.
+
+## Root cause 1: diary-drafts.spec.ts:854 (the actual trigger)
+
+`test.slow()` triples the desktop project's **overall per-test timeout** (15s → 45s).
+This is a single shared ceiling across every step in the test PLUS the `finally`-block
+cleanup — it is NOT "3x budget per step," and it does NOT triple `actionTimeout` or
+`expect.timeout` (a misconception baked into the original code's comments). The test
+paired `test.slow()` (45s total) with FOUR separate explicit step timeouts of
+30_000-45_000ms each (`waitForResponse`, `waitForURL`, two `expect(...).toBeVisible()`
+calls) — any single one of which could alone consume the entire budget. Under heavy
+8-worker CI load the cumulative real elapsed time across these steps regularly ate the
+whole 45s, leaving nothing for the `finally` block's `deleteDiaryEntryViaApi()` call —
+which is what actually threw ("apiRequestContext.delete: Test timeout of 45000ms
+exceeded"), masking the true cause (slow steps, not a slow delete).
+
+**Fix** (commit on `fix/1829-e2e-shard3-flakes`): replaced `test.slow()` with
+`test.setTimeout(60_000)` (explicit absolute budget, more generous and self-documenting
+than a 3x multiplier), reduced the 4 oversized step timeouts to 15_000ms each
+(proportionate "something is actually stuck" ceilings, not budget reservations), and —
+the generalizable pattern — added `testInfo.setTimeout(testInfo.timeout + 15_000)` as
+the FIRST line of the `finally` block before cleanup. This call is **additive to the
+current remaining deadline**, not a reset to an absolute value, so it always grants
+15s of guaranteed headroom for cleanup regardless of how much of the original budget
+the test body already burned. Wrapped the delete in `try/catch` (best-effort cleanup;
+an orphaned draft in the ephemeral CI DB is harmless, a masked test outcome is not).
+Applied the same defensive pattern to `document-linking.spec.ts:369`'s two-item
+cleanup even though it wasn't the root cause, since it's cheap insurance and directly
+named in the issue.
+
+## Root cause 2: diary-r2-uat.spec.ts:599 (secondary, real race)
+
+Both this test and `Scenario 9` mock `**/api/diary-entries*` with an ALWAYS-EMPTY
+paginated response (`makePaginatedEmpty()`). `DiaryPage.waitForLoaded()` races
+`timeline`/`emptyState`/`errorBanner` visibility — but since the empty state is
+already visible from a *prior* transition and never changes, `.waitFor({state:
+'visible'})` on an already-visible element resolves **instantly**, without
+synchronizing to the new request/response at all. Scenario 10 clicks "All" then
+"Manual" (two transitions before the assertion); after the "All" click,
+`waitForLoaded()` is a no-op, so the test proceeds to reset a `requests[]` capture
+array and register a fresh `waitForResponse` for the "Manual" click's response —
+but the "All" click's own response can still be in flight and arrive AFTER the
+reset, matching the loose `waitForResponse` predicate (any 200 response containing
+`/api/diary-entries`) and resolving it *before* the real "Manual" request/response
+pair occurs. Result: `requests[]` is empty when read → `expect(lastRequest).toBeDefined()`
+fails intermittently. Scenario 9 does NOT have this race because its `requests[]`
+reset happens right after the very first (`goto`) transition, whose response is
+provably not-yet-arrived when `waitForLoaded()` resolves (nothing was visible before
+first navigation).
+
+**Fix**: explicitly `await page.waitForResponse(pred)` for the "All" transition's own
+response (registered before the click) before resetting/registering for "Manual", and
+derive `lastRequest`/`typeParam` directly from the resolved `Response` object's URL
+instead of a side-channel mutable array — eliminates the race entirely rather than
+narrowing the window.
+
+## `maxFailures` / retry accounting — proof it already tolerates one flaky test
+
+Verified against installed `testcontainers`... no wait, against `@playwright/test@1.61.1`
+source (`packages/playwright/src/runner/dispatcher.ts`, `_reportTestEnd`):
+
+```ts
+// Test is considered failing after the last retry.
+if (test.outcome() === 'unexpected' && test.results.length > test.retries)
+  ++this._testRun.failedTestCount;
+```
+
+`maxFailures` only increments once a test has **exhausted all configured retries**
+AND is still `'unexpected'`. A test that fails attempt 1 but passes on retry
+("flaky") never counts. **AC #3 of issue #1829 ("should fail-fast tolerate one
+retry-passing test before cancelling?") is already satisfied by Playwright's design —
+no config change was needed.** `maxFailures: 1` in `e2e/playwright.config.ts` was left
+as-is; only the comment was expanded to document this (with a source citation) so a
+future engineer doesn't re-litigate it. Root-causing #1 above is the real fix — once
+that test reliably passes on attempt 1 (or at worst passes on retry), fail-fast won't
+trip and the other two tests stop showing collateral/independent failures.
+
+## Playwright config values (verified 2026-07-08, corrects a stale prior note)
+
+- Global `use`: `actionTimeout: 5_000`, `navigationTimeout: 10_000`.
+- Desktop: test `timeout: 15_000` (top-level default), `expect: {timeout: 7_000}`.
+- Tablet/Mobile: test `timeout: 30_000`, `expect: {timeout: 10_000}`, action/nav `10_000`.
+- `retries: 1` on CI, `maxFailures: 1` only when `E2E_FAIL_FAST=1` (main-targeted PRs).
+
+## Incidental fix: testcontainers API drift (e2e/containers/cornerstoneContainer.ts)
+
+While attempting a real containerized verification run of this fix, discovered
+`cornerstoneContainer.ts`'s `HTTP_PROXY`/`HTTPS_PROXY` branch called
+`.withCopyFileToContainer(caPath, target)` (singular) — this method does not exist in
+the installed `testcontainers@12.0.4` (renamed to `.withCopyFilesToContainer([{source,
+target}])`, plural, array-based, confirmed by reading `node_modules/testcontainers/build/generic-container/generic-container.d.ts`
+and a `node -e` runtime probe). This branch only executes when `HTTP_PROXY`/`HTTPS_PROXY`
+env vars are set, which CI never sets — so the bug was dormant in CI and only surfaces
+in a local sandbox that sits behind an HTTP(S) proxy requiring a trusted CA (like this
+one). Fixed as a one-line incidental change. Full containerized verification still
+wasn't possible in this sandbox after that fix: building the `cornerstone:e2e` image
+requires `dhi.io` (Docker Hardened Images) registry credentials not available here
+(`401 Unauthorized` pulling `dhi.io/node:24-alpine3.23`) — this is a credentials/
+environment limitation, not a code issue. **If you hit `withCopyFileToContainer is not
+a function` in a future session, this is already fixed** — don't re-diagnose, just
+check whether a newer `testcontainers` bump reintroduced /renamed the API again.
diff --git a/.claude/agent-memory/e2e-test-engineer/general-e2e-patterns.md b/.claude/agent-memory/e2e-test-engineer/general-e2e-patterns.md
@@ -21,9 +21,16 @@ metadata:
 
 ## Viewport timeouts (Playwright projects)
 
-- Desktop: `timeout: 10_000`, default action/expect timeouts (Playwright default 30s / no override).
-- Tablet & Mobile: `timeout: 60_000`, `expect/action/navigationTimeout: 15_000`.
+Verified against `e2e/playwright.config.ts` 2026-07-08 (previous note here was stale/wrong — corrected):
+
+- Global `use`: `actionTimeout: 5_000`, `navigationTimeout: 10_000`.
+- Desktop: test `timeout: 15_000` (top-level default, no per-project override), `expect: {timeout: 7_000}`.
+- Tablet & Mobile: test `timeout: 30_000` (2x desktop), `expect: {timeout: 10_000}`, `actionTimeout: 10_000`, `navigationTimeout: 10_000`.
+- `maxFailures: 1` only on main-targeted PRs (`E2E_FAIL_FAST=1`); `retries: 1` on CI. See [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md) for confirmed `maxFailures`/retry accounting semantics.
 - Never hardcode a lower `{timeout:N}` in a POM `waitFor()` call — it silently overrides the project-level tablet/mobile timeout.
+- **`test.slow()` triples the OVERALL per-test timeout only** (desktop 15s→45s) — it does NOT triple `actionTimeout`/`expect.timeout`. That total is a single shared ceiling across every step in the test PLUS any `finally`-block cleanup; it is not a "budget per step." Never pair `test.slow()` with several individual step timeouts anywhere near or above the tripled total (e.g. three separate `{timeout: 45_000}` waits inside a test whose own timeout is 45_000) — under CI load the cumulative real time silently eats the whole budget and the failure surfaces at whatever call happens to be in flight when the deadline fires (often cleanup), misdirecting triage. See [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md).
+- **Guarantee teardown time regardless of test-body duration**: in a `finally` block (or `afterEach`), call `testInfo.setTimeout(testInfo.timeout + N)` right before cleanup — this is additive to the *current remaining* deadline (not a reset to an absolute value), so it always grants `N` ms of headroom for cleanup no matter how much of the original budget the test body already consumed. Requires adding `testInfo` as the test callback's second parameter: `async ({ page }, testInfo) => {...}`. Combine with `try/catch` around the cleanup call itself so a still-slow/failed delete doesn't throw past the (now-extended) deadline or mask the test's real outcome.
+- **A route-mocked response that ALWAYS returns an empty page defeats `waitForLoaded()`-style "timeline/empty-state/error-banner visible" race helpers** — if the empty state was already visible from a *prior* transition, `.waitFor({state:'visible'})` on it resolves instantly without synchronizing to the new request. This breaks the common "click filter chip → `waitForLoaded()` → reset a `requests[]` capture array → register `waitForResponse` for the next click" pattern: a late-arriving response from the *previous* click can satisfy the next `waitForResponse` predicate before the real new request/response pair occurs, leaving the reset `requests[]` array empty when read. Fix: explicitly `await page.waitForResponse(pred)` for each transition's own response (don't rely on the UI-loaded signal to gate a network-timing-sensitive reset), and derive request/response data straight from the resolved `Response` object rather than a side-channel array.
 
 ## Strict-mode / selector anti-patterns
 

diff --git a/.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md b/.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md
@@ -7,7 +7,7 @@ metadata:
 
 ## Currently open / unresolved
 
-- **Diary shard-3 flakiness NOT fully resolved by fix PRs #1790/#1792/#1793** (triaged 2026-07-07): on beta PR #1803, `diary-r2-uat.spec.ts:599` "Manual mode API parameter (Scenario 10)" fails with `expect(lastRequest).toBeDefined()` (empty-requests-array race, a NEW failure mode); `diary-drafts.spec.ts:854` "Draft card click navigates to edit page (Scenario 14)" fails via `apiRequestContext.delete: Test timeout of 45000ms exceeded` in teardown (also a NEW failure mode vs. the original nav-timing flake). Same shard-3 pattern seen across PRs #1780/#1782/#1788/#1803 (2026-06-22 → 2026-07-06) — pre-existing, not caused by any single story. `gh run view --log`/`--log-failed` returns empty for these matrix jobs; use `gh api repos/.../actions/jobs/<id>/logs` instead. Non-blocking on beta (E2E Gates informational there) but needs dedicated follow-up — flag to orchestrator before picking up diary work.
+(none — see Resolved section for the #1829 shard-3 fix)
 - `i18n/i18n.spec.ts` "German text does not overflow navigation sidebar on desktop" — pre-existing locale-init race, needs separate investigation.
 - `i18n.spec.ts` "Key page headings render in German" — intermittent ~10-20%: concurrent worker `afterEach(resetToEnglish)` races with another test's `setLanguage('de')`. Pre-existing.
 - `i18n-categories.spec.ts` "German locale: Manage trades tab shows 'Sanitär'..." — intermittent, locale doesn't initialize before English page renders. Pre-existing (seen before PR #1186 too).
@@ -19,7 +19,8 @@ metadata:
 
 ## Resolved / fixed
 
-- Diary Scenario 14 (`diary-drafts.spec.ts` draft-card-click-navigates) — was a persistent flake even after an earlier partial fix (PR #1671); root-caused and properly fixed via `fix/diary-scenario14-e2e-flake` (register `waitForResponse` before the click, not after). **Note (2026-07-07): recurred on shard 3 with a DIFFERENT failure signature** — see "Currently open" above; the original nav-timing bug is fixed but a new teardown-timeout flake has appeared since.
+- **Issue #1829 (2026-07-08)** shard-3 diary flakes on main-targeted PRs: `diary-drafts.spec.ts:854` (Scenario 14) was genuinely failing on both attempt + retry — `test.slow()`'s 45s total test budget was being exhausted by 4 oversized nested step timeouts (30-45s each) before reaching cleanup, surfacing as `apiRequestContext.delete: Test timeout exceeded`. Fixed via `test.setTimeout(60_000)` + proportionate 15s step ceilings + `testInfo.setTimeout(testInfo.timeout + 15_000)` guaranteed-cleanup-time pattern in `finally`. `diary-r2-uat.spec.ts:599` (Scenario 10) had a genuine secondary race (always-empty mock defeats `waitForLoaded()`, letting a stale prior response satisfy the next `waitForResponse`) — fixed by awaiting each transition's own response explicitly and reading straight off the resolved `Response` object. `document-linking.spec.ts:369` was NOT an independent bug — confirmed via job logs it was cancelled mid-flight (1.6s runtime) as `maxFailures:1` collateral once drafts:854 exhausted its retries; no test-logic fix needed, just added the guaranteed-cleanup-time pattern defensively. Full root cause + `maxFailures`/retry-accounting proof (Playwright source citation) in [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md). AC #3 (fail-fast retry tolerance) was already satisfied by Playwright's built-in behavior — no config change needed, only documented.
+- Diary Scenario 14 (`diary-drafts.spec.ts` draft-card-click-navigates) — was a persistent flake even after an earlier partial fix (PR #1671); root-caused and properly fixed via `fix/diary-scenario14-e2e-flake` (register `waitForResponse` before the click, not after). Recurred 2026-07-07 with a DIFFERENT failure signature (teardown-timeout, not nav-timing) — see #1829 entry above for the follow-up fix.
 - `invoice-budget-line-create-and-link.spec.ts` Scenarios 1–4 — REAL production regression from PR #1566 (`eagerLinkInvoice:false`), filed as bug #1611. Tests were NOT weakened; they assert correct behavior.
 - Shard 3 promotion blocker (`budget-source-filter.spec.ts`, 2026-06-12): "Rapid debounce coalesces requests" flaked on strict request-count assertions vs. CI click serialization beyond the 50ms debounce (fixed PR #1665); "Perspective toggle changes Cost value" flaked on reading `textContent()` immediately after a radio click without waiting for React re-render on WebKit (fixed PR #1666). **General fix pattern**: after any click triggering a React state update, `await expect(locator).not.toHaveText(previousValue)` before `textContent()` — never read immediately post-click on WebKit. Remove `page.on('request', ...)` listeners you add (persist for the page's lifetime otherwise); prefer state assertions (`aria-pressed`, URL params) over raw request counting.
 - `dashboard.spec.ts:566` "Customize button appears when card dismissed" — fixed PR #1445 (`expect.poll` for preference state). Was issue #1431.