Commit Graph

1560 Commits

Author SHA1 Message Date
Marcel
b2e31c3c1b refactor(observability): lower trace sample rate, add DSN comment, improve status visibility
- Lower tracesSampleRate from 1.0 to 0.1 in both hooks (errors still captured
  at 100%; trace volume reduced for self-hosted GlitchTip on shared VPS)
- Add comment explaining VITE_SENTRY_DSN is a write-only ingest key, safe in
  client bundle — prevents accidental rotation as if it were a password
- Restore HTTP status code prominence: text-4xl font-bold (was text-xs text-ink-3)
- Add min-w-[44px] to copy button for WCAG 2.2 minimum touch target

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 10:27:01 +02:00
Marcel
9e23620072 refactor(observability): add hooks.server.ts to coverage include in vite.config.ts
The handleError callback in hooks.server.ts is now gated by the 80% branch
coverage threshold along with the rest of the server-side logic.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 10:26:03 +02:00
Marcel
af42113fca test(observability): add hooks.client.test.ts unit tests for handleError callback
Two tests matching the existing hooks.server.test.ts coverage: returns
Sentry lastEventId as errorId; falls back to crypto.randomUUID when
lastEventId returns undefined.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 10:25:30 +02:00
Marcel
c779ec59f9 feat(observability): guard navigator.clipboard and handle rejection in copyId
Adds availability guard (navigator.clipboard may be undefined in non-HTTPS
contexts) and a rejection handler so clipboard-denied errors are silently
caught rather than becoming unhandled promise rejections. Tests cover the
success feedback and the silent-failure path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 10:24:35 +02:00
Marcel
59b18039ed refactor(observability): remove console.log from tags proxy and enforce no-console lint rule
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 09:45:49 +02:00
Marcel
96ea7e6815 feat(observability): redesign +error.svelte with errorId display and copy button
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 09:44:32 +02:00
Marcel
dff81f7bfb feat(observability): add handleError callback to hooks.client.ts returning errorId
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 09:41:58 +02:00
Marcel
a9c82ec481 feat(observability): add handleError callback to hooks.server.ts returning errorId
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 09:41:24 +02:00
Marcel
97aa372094 feat(observability): add App.Error interface with errorId to app.d.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-17 09:40:12 +02:00
Marcel
23515b8542 fix(eslint): remove projectService from Svelte parser — restores fast lint
Some checks failed
CI / OCR Service Tests (pull_request) Has been cancelled
CI / Backend Unit Tests (pull_request) Has been cancelled
CI / fail2ban Regex (pull_request) Has been cancelled
CI / Compose Bucket Idempotency (pull_request) Has been cancelled
CI / Unit & Component Tests (pull_request) Has been cancelled
CI / Unit & Component Tests (push) Successful in 3m23s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 2m37s
CI / fail2ban Regex (push) Successful in 44s
CI / Compose Bucket Idempotency (push) Successful in 1m1s
nightly / deploy-staging (push) Failing after 2m33s
5646e739 added svelte-kit sync before lint so .svelte-kit/tsconfig.json
always exists. This activated projectService: true for every run, which
builds the full TypeScript language service for all .svelte files and
caused CI lint to take 7+ minutes.

None of the rules in the Svelte-specific block need type information —
they are all AST-selector-based no-restricted-syntax checks. Removing
projectService restores the previous fast path without losing any lint
coverage.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-15 20:08:52 +02:00
Marcel
00a8731cdd fix(frontend): add sentrySvelteKit Vite plugin for source map upload
Some checks failed
CI / Unit & Component Tests (pull_request) Successful in 6m19s
CI / OCR Service Tests (pull_request) Successful in 40s
CI / Backend Unit Tests (pull_request) Failing after 25m0s
CI / fail2ban Regex (pull_request) Successful in 2m13s
CI / Compose Bucket Idempotency (pull_request) Successful in 2m3s
Adds the sentrySvelteKit() Vite plugin as the first plugin in vite.config.ts.
When SENTRY_AUTH_TOKEN is set at build time, source maps are uploaded to
GlitchTip so error stack traces show original TypeScript source and line number.
When SENTRY_AUTH_TOKEN is absent (CI, dev builds), upload is disabled via
autoUploadSourceMaps: false — the build succeeds normally.

Resolves Felix's review blocker on PR #591.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-15 06:23:34 +02:00
Marcel
b4e6e4ca2a feat(frontend): integrate @sentry/sveltekit for browser and SSR error reporting
Some checks failed
CI / Unit & Component Tests (pull_request) Successful in 6m37s
CI / OCR Service Tests (pull_request) Successful in 41s
CI / Backend Unit Tests (pull_request) Failing after 24m43s
CI / fail2ban Regex (pull_request) Successful in 2m18s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m57s
Adds @sentry/sveltekit to hooks.client.ts and hooks.server.ts.
When VITE_SENTRY_DSN is unset (default), Sentry is fully disabled.
When set to a GlitchTip JavaScript project DSN, browser exceptions
and SSR handleError events are forwarded automatically.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-15 06:15:34 +02:00
Marcel
33c738db3b fix(docker): skip postinstall in production image
Some checks failed
CI / Unit & Component Tests (pull_request) Successful in 3m9s
CI / OCR Service Tests (pull_request) Successful in 15s
CI / Backend Unit Tests (pull_request) Successful in 4m31s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 59s
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / Unit & Component Tests (push) Has been cancelled
The production stage runs npm ci --omit=dev to install runtime deps for
the pre-built SvelteKit app. The postinstall script calls patch-package,
which is a devDependency, so it is absent and causes exit code 127.

--ignore-scripts is the correct npm-native fix: no lifecycle scripts are
needed when installing into a pre-built image.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:42:52 +02:00
Marcel
62c807b7fe fix(invites): resolve svelte-check warnings in UserGroupsSection and page.server.test
All checks were successful
CI / Unit & Component Tests (push) Successful in 3m11s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m22s
CI / fail2ban Regex (push) Successful in 39s
CI / Compose Bucket Idempotency (push) Successful in 56s
Use untrack() for intentional one-time prop seed in UserGroupsSection.
Add explicit LoadData type alias in page.server.test to avoid void|Record<string,any> union.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
4994d28a20 feat(invites): show empty state when no groups exist in invite form
When groups load successfully but the list is empty, render a quiet
"Keine Gruppen vorhanden." message rather than a blank section that
leaves users uncertain whether groups failed to load.

Adds admin_new_invite_no_groups i18n key to de/en/es.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
24a398a0d8 fix(invites): i18n legend + touch target in UserGroupsSection
- legend uses m.admin_new_invite_groups() instead of hardcoded "Gruppen"
  so screen readers announce the correct string in en/es locales
- label gets min-h-[44px] for WCAG 2.2 touch target compliance
- add test asserting fieldset accessible name comes from i18n key
- add test documenting empty-groups-no-error renders no checkboxes/banner

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
0a5d4fb950 feat(errors): add GROUP_NOT_FOUND error code + i18n keys
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
e4303baa40 test(invites): import real +page.server module via vi.mock env
Replace hand-copied load/action replicas with direct imports of the
real module. Mock $env/dynamic/private so the tests cover the actual
production code paths, not a duplicate that can drift.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
46c8d4553b fix(invites): add role="alert" to groups-load-error banner
Screen readers now announce the amber warning when it appears after
the form expands, without requiring the user to navigate to it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
3fc0ec95ef fix(invites): make group checkboxes writable — $derived → $state
bind:group requires a writable $state variable; $derived is read-only
in Svelte 5, so every click was silently reset to unchecked, making
the group picker non-functional.

Also wraps checkboxes in <fieldset>/<legend> for WCAG 1.3.1 compliance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
510fa5e398 feat(invites): group picker in new-invite form
- load() fetches /api/groups in parallel with /api/invites; returns
  sorted groups array and groupsLoadError for partial failures
- create action forwards groupIds[] to POST /api/invites so invited
  users are placed in the selected groups on registration
- +page.svelte: group checkboxes via UserGroupsSection inside the form;
  amber warning banner when groups could not be loaded
- page.svelte.test.ts: groups checkboxes + warning banner tests
- page.server.test.ts: parallel fetch, sorting, error fallback,
  groupIds in POST body

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
75453bed51 feat(frontend): add GROUP_HAS_ACTIVE_INVITES error code + i18n keys
Adds the error code to the ErrorCode union and getErrorMessage() switch.
Adds admin_new_invite_groups, admin_invite_groups_load_error, and
error_group_has_active_invites to all three locale files (de/en/es).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 19:26:53 +02:00
Marcel
0f4c844002 fix(admin/system): address second-round review concerns
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m9s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Successful in 4m29s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 55s
CI / Unit & Component Tests (push) Successful in 3m8s
CI / OCR Service Tests (push) Successful in 16s
CI / Backend Unit Tests (push) Successful in 4m25s
CI / fail2ban Regex (push) Successful in 38s
CI / Compose Bucket Idempotency (push) Successful in 55s
- Extract ImportStatus type to types.ts — removes duplication across
  +page.svelte, ImportStatusCard.svelte, and test file (Felix blocker)
- Fix H2 to match CLAUDE.md card pattern: text-xs uppercase tracking-widest
  text-ink-3 mb-5 (Leonie blocker 1)
- Add font-sans to RUNNING and DONE status labels (Leonie blocker 2)
- Add data-testid="processed-count" to count elements in both states
- Replace document.querySelector with locator API in spinner tests
- Tighten getByText('7') to getByTestId('processed-count') (Felix/Sara)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 16:27:33 +02:00
Marcel
b0cf35cf06 fix(test): replace toBeAttached() with querySelector not-null check for spinner
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m10s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Successful in 4m25s
CI / fail2ban Regex (pull_request) Successful in 40s
CI / Compose Bucket Idempotency (pull_request) Successful in 55s
toBeAttached() is not in the vitest-browser matcher set; toBeVisible() was
previously ruled out because the spinner is 0x0 px. Mirror the querySelector
pattern already used for the negative case in the same file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 15:24:27 +02:00
Marcel
0d934a1b44 fix(test): use m() calls and toBeAttached() in ImportStatusCard tests
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m36s
CI / OCR Service Tests (pull_request) Successful in 15s
CI / Backend Unit Tests (pull_request) Successful in 4m21s
CI / fail2ban Regex (pull_request) Successful in 37s
CI / Compose Bucket Idempotency (pull_request) Successful in 56s
CI Chromium runs with German locale so hardcoded English strings like
'No spreadsheet file found.' never matched. Use m.admin_system_import_*()
to assert whatever locale the browser resolves to.

Spinner test used toBeVisible() on an empty <span> whose dimensions come
entirely from Tailwind CSS. Without layout CSS the span is 0×0 and fails
the visibility check; toBeAttached() asserts DOM presence, which is the
right semantic here.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 15:14:50 +02:00
Marcel
f4bda546a0 fix(test): update import-status test mocks and imports for statusCode-based i18n
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 4m6s
CI / OCR Service Tests (pull_request) Successful in 17s
CI / Backend Unit Tests (pull_request) Successful in 4m23s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Successful in 57s
Three test files were written against the old API shape (raw `message` field) before
the statusCode i18n field was introduced, or used the wrong `expect` import path:

- ImportStatusCard.svelte.test.ts: `@vitest/browser/context` does not export `expect`
  in this project's Vitest setup — use `vitest` like every other test file.
- page.svelte.spec.ts: FAILED mock lacked `statusCode`; assertion matched old German
  raw message instead of the i18n string for IMPORT_FAILED_NO_SPREADSHEET.
- page.svelte.test.ts: same pattern — mock lacked `statusCode`; assertion checked for
  raw backend string "database error" instead of the rendered i18n text.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 14:37:04 +02:00
Marcel
b7744667f2 fix(admin/system): address review concerns in ImportStatusCard
- Remove dead `message` field from both frontend ImportStatus types
  (field is now @JsonIgnore'd on the backend)
- Extract failure message ternary into `$derived` — business logic off
  the template (Felix)
- Add motion-reduce:animate-none to spinner — WCAG 2.1 SC 2.3.3 (Leonie)
- Replace text-green-600 with text-green-800 — WCAG AA contrast 6.1:1
  on bg-green-50 (Leonie)
- Add min-h-[44px] to all three buttons — WCAG 2.2 44px touch target (Leonie)
- Add 6 missing tests: IMPORT_FAILED_INTERNAL path, IDLE state text,
  null importStatus, ontrigger called on DONE/FAILED/IDLE buttons (Sara)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 14:37:04 +02:00
Marcel
375fd3893c feat(admin/system): extract ImportStatusCard — spinner, text-base count, statusCode i18n
Extracts the mass-import block from +page.svelte into ImportStatusCard.svelte.

Changes per the three UX fixes from issue #533:
- RUNNING: animated spinner (animate-spin) + processed count at text-base;
  auto-poll at 2 s was already in place
- DONE: processed count at text-base, label at text-xs uppercase tracking-widest
- FAILED: maps statusCode (IMPORT_FAILED_NO_SPREADSHEET / IMPORT_FAILED_INTERNAL)
  to Paraglide messages — no raw German backend string rendered

Adds vitest-browser tests covering spinner visibility, count display,
and per-statusCode FAILED message selection.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 14:37:04 +02:00
Marcel
c5d482bead feat(i18n): add structured import failure keys; split DONE display
Replaces the {message} interpolation (raw German backend string) with
two distinct error keys: IMPORT_FAILED_NO_SPREADSHEET and
IMPORT_FAILED_INTERNAL. Also removes the {count} parameter from the
done message and adds admin_system_import_status_done_label so the
processed count can be rendered separately at text-base size.

All three locales (de / en / es) updated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 14:37:04 +02:00
Marcel
d78ee4397b devops(ci): add testTimeout + hookTimeout to browser vitest config
Some checks failed
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / Unit & Component Tests (push) Has been cancelled
testTimeout: 30_000 causes Vitest to fail a hanging browser test
within 30 s when Chromium crashes mid-load instead of silently
occupying the CI slot for 14+ min.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 14:25:37 +02:00
Marcel
2ca8428be4 refactor(test): hoist SubmitFn to file-level type in unsaved-guard specs
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 4m9s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 5m8s
CI / fail2ban Regex (pull_request) Successful in 48s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m3s
CI / Unit & Component Tests (push) Successful in 3m24s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m24s
CI / fail2ban Regex (push) Successful in 40s
CI / Compose Bucket Idempotency (push) Successful in 59s
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 12:12:08 +02:00
Marcel
6fffc06c28 fix(test): allow extra result properties in enhance callback type
Some checks failed
CI / Unit & Component Tests (pull_request) Successful in 3m8s
CI / OCR Service Tests (pull_request) Successful in 17s
CI / fail2ban Regex (pull_request) Successful in 48s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
CI / Backend Unit Tests (pull_request) Failing after 17m19s
Use [key: string]: unknown index signature so TS does not reject the
extra fields (location, status) passed to the redirect/failure result
in the spec helpers.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 12:09:14 +02:00
Marcel
ffcb901376 fix(admin): clear unsaved-changes guard before redirect on users/new
Mirror the groups/new fix: replace inline beforeNavigate/isDirty with
createUnsavedWarning() + UnsavedWarningBanner and add an enhance callback
that calls clearOnSuccess() before update() on redirect results.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 12:09:14 +02:00
Marcel
30469e74c9 fix(admin): clear unsaved-changes guard before redirect on groups/new
Use createUnsavedWarning() + UnsavedWarningBanner to replace the inline
beforeNavigate/isDirty pattern, and add an enhance callback that calls
clearOnSuccess() before update() so the guard is disarmed before
SvelteKit's internal goto() fires on a redirect result.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 12:09:14 +02:00
Marcel
f727429699 fix(ci): run client coverage even when server coverage fails
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
Replace && with ; in test:coverage so the client vitest run is not
short-circuited when the server run exits non-zero (e.g. threshold
violation or test failure). Without this the upload-artifact step
only ever sees coverage/server.

Also updates the stale CLAUDE.md comment that said server-only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 11:07:34 +02:00
Marcel
e268e2dbca fix(tests): use native element clicks in layout dropdown spec
Some checks failed
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CDP-based Playwright clicks (locator.click()) do not reliably trigger
Svelte 5 onclick handlers — documented in commit 0c765d81 which fixed
13 other specs. The layout dropdown tests were missed in that pass.

Applies the same pattern: ((await locator.element()) as HTMLElement).click()
for button interactions, and native KeyboardEvent dispatch for the Escape
test (dispatched on the button so it bubbles to the parent div's onkeydown).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 11:07:22 +02:00
Marcel
118100e58d chore(coverage): drop client branches threshold 80→75, add ADR-013
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / OCR Service Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Failing after 2m35s
CI / OCR Service Tests (pull_request) Successful in 16s
CI / Backend Unit Tests (pull_request) Successful in 4m21s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Compose Bucket Idempotency (pull_request) Failing after 11s
Branches gate was blocking CI at 75% measured coverage. The 80% floor
suffers Istanbul parent/child denominator coupling (long-tail grind, per
#496) that makes the remaining gap disproportionately costly to close.
Drop branches to 75 to match current state; leave lines/functions/
statements at 80. ADR-013 documents the rationale and the ratchet rule
for raising the gate back incrementally.

Closes #556

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 08:54:59 +02:00
Marcel
7fc1295dc0 build(patch-package): backport vitest PR #10267 for browser-playwright birpc race
Installs patch-package (^8.0.0) and a postinstall script, then applies
the diff from vitest PR #10267 against @vitest/browser-playwright@4.1.0.

What the patch changes (in dist/index.js):

- createPredicate(sessionId, url) → createPredicate(url): factory becomes
  pure, returns { url, predicate } instead of mutating sessionIds /
  idPreficates as a side-effect.
- sessionIds value type: array → Set (deduplicates resolved URLs).
- register handler now looks up any existing predicate for the
  (sessionId, resolvedUrl) pair and unroutes it BEFORE installing the
  new route. This is the actual race fix: without it, the second
  vi.mock for a duplicate-id leaks an orphan Playwright route that
  fires after birpc closes.
- clear handler iterates the Set via spread.

Why this matters even though Layer 1 normalised the only known duplicate
in our suite: every future vi.mock call is a class of race we shouldn't
have to think about. The patch closes the upstream gap at the
route-handler level, so a contributor reintroducing the duplicate-id
pattern can't reopen the race.

When to remove: when @vitest/browser-playwright ships a release
containing PR #10267. Delete patches/@vitest+browser-playwright+4.1.0.patch
and the postinstall hook (or keep the hook if other patches accumulate).

Refs: #553 · vitest-dev/vitest#9957 · vitest-dev/vitest#10267

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 08:08:28 +02:00
Marcel
0cf4a488bb test(meta): add duplicate-id vi.mock detector under __meta__/
Scans every src/**/*.svelte.{spec,test}.ts file for vi.mock first-arg
strings, canonicalises each by stripping a trailing .js/.ts after
.svelte, groups by canonical id, and fails if any canonical id is
referenced under two or more distinct raw spellings.

Mirrors the shape of src/__meta__/no-async-mock-factories.test.ts:
source-text regex scan (no AST parser dependency), red/green self-test
fixtures inline, then one corpus assertion that the whole suite is
clean.

This is the in-suite defence-in-depth layer for the duplicate-id birpc
race named in ADR-012 / #553 and fixed upstream by vitest PR #10267.
Harder to disable than ESLint (cross-file invariant ESLint cannot
express anyway) and harder to scope around than a CI grep.

Refs: #553

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 08:06:14 +02:00
Marcel
9030a7d031 test(confirm-service): normalise vi.mock spelling and remove duplicate-id mocks
Five test files mocked $lib/shared/services/confirm.svelte under BOTH
spellings (.svelte and .svelte.js) within the same file; two more mocked
only the .svelte.js form. Both resolve to the same module URL but register
two distinct Playwright route handlers in @vitest/browser-playwright. The
cleanup logic only removes one, leaving an orphan that fires when the next
session loads the module — crashing the run with
"[birpc] rpc is closed, cannot call resolveManualMock".

This is the exact trigger fixed upstream by vitest PR #10267 (issue #9957).
Normalise every confirm.svelte mock to the no-extension form, matching
production imports and the source file basename (confirm.svelte.ts).

After this commit: 8 confirm.svelte mocks across 8 spec files, all under
one canonical ID. A meta-test (next commit) prevents the duplicate-id
pattern from reappearing.

Refs: #553 · vitest-dev/vitest#9957 · vitest-dev/vitest#10267

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 08:03:43 +02:00
Marcel
feadf372a0 refactor(confirm-service): normalise import spelling to no-extension form
Production code referenced $lib/shared/services/confirm.svelte under two
spellings — 4 files with the .js extension and one without. Standardise on
the no-extension form to match Svelte 5 rune-module convention and the
source file basename (confirm.svelte.ts).

Why this matters: vitest browser mode's @vitest/browser-playwright resolves
both spellings to the same module URL but registers a separate Playwright
route per spelling. The route-cleanup logic only unregisters the latest,
leaving an orphan that crashes the next session with
"[birpc] rpc is closed, cannot call resolveManualMock". Fixed upstream in
vitest PR #10267 (merged, not yet released). Normalising the spelling
removes the trigger from our side.

Refs: #553. Companion test-file changes follow in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 08:02:26 +02:00
Marcel
edde9292e6 test(setup): also disable data-sveltekit-preload-code in browser tests
Some checks failed
CI / Backend Unit Tests (push) Successful in 4m17s
CI / fail2ban Regex (push) Successful in 39s
CI / Compose Bucket Idempotency (push) Failing after 11s
CI / Unit & Component Tests (push) Failing after 2m31s
CI / OCR Service Tests (push) Successful in 17s
Hover-prefetch has two surfaces in SvelteKit:
- data-sveltekit-preload-data (route loader data)
- data-sveltekit-preload-code (route JS chunks)

The original fix turned off only the loader-data side. Route-code chunks
prefetched on hover can also include manually-mocked module URLs; an
in-flight code prefetch landing after iframe teardown hits the same
Playwright route handler that resolves manual mocks, raising the
unhandled rejection. Disable both surfaces.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:32:03 +02:00
Marcel
5afebde382 refactor(eslint): ban async vi.mock factories with dynamic import in body
Generalise the no-restricted-syntax rule from the literal pdfjs-dist
selector (added in #535) to also catch the underlying mechanism named in
ADR-012 / #553: any `vi.mock(..., async () => { ... await import(...)
... })` produces a late birpc roundtrip during worker teardown.

Selector: vi.mock CallExpression whose second argument is an
ArrowFunctionExpression with async=true and whose subtree contains an
AwaitExpression > ImportExpression. Both rules coexist — the literal
pdfjs-dist rule still enforces the libLoader prop injection pattern
(catches sync forms too); the new rule enforces the sync-factory
invariant universally.

Demonstrated by planting a synthetic offender locally and watching
ESLint flag it with the new rule's message.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:09:07 +02:00
Marcel
636d61a81b test(meta): scan src/**/*.svelte.{test,spec}.ts for async vi.mock factories
In-suite belt-and-braces detector for the birpc teardown race named in
ADR-012 / #553. Catches `vi.mock(<arg>, async ... { ... await import(...)
... })` in any browser spec on every vitest invocation — the layer hardest
to disable or scope around (ESLint can be silenced; CI grep runs only in
CI; this test runs whenever the suite runs).

Demonstrated red→green by planting a synthetic offender locally and
watching the live-scan assertion fail; removing the offender returned it
to green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:05:43 +02:00
Marcel
3c9e40ca71 test(setup): disable SvelteKit hover prefetch in browser-mode runs
Hover-prefetch fires real fetch requests for route loader chunks; those
requests go through the same Playwright route handler that serves mocked
modules. An in-flight prefetch landing after iframe teardown can hit the
handler with a closed birpc channel, raising an unhandled rejection that
exits the run with code 1 even when every individual test was green.

Add `src/test-setup.ts` that sets `document.body.dataset.sveltekitPreloadData
= 'off'` and wire it via `setupFiles` in both `vite.config.ts` (client
project) and `vitest.client-coverage.config.ts` (Istanbul coverage config).
Add `src/__meta__/browser-preload-disabled.svelte.test.ts` asserting the
setup ran. Zero production impact.

Issue #553 secondary trigger.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:02:57 +02:00
Marcel
9f1b8b4215 fix(enrichment-block): migrate $app/stores → $app/state to eliminate birpc race
The async vi.mock factory in EnrichmentBlock.svelte.spec.ts performed an
`await import(...)` in its body — the same mechanism #535/#546 fixed for
pdfjs-dist. Issue #553: when Chromium's playwright route handler fetches
the mocked module after the worker's birpc channel has closed, the
factory's RPC roundtrip raises `[birpc] rpc is closed, cannot call
"resolveManualMock"` and the run exits 1.

Migrate EnrichmentBlock from the deprecated `$app/stores.navigating`
(store) to the modern `$app/state.navigating` (reactive proxy). The
spec uses vi.hoisted + a sync vi.mock factory with a getter that defers
the read — no dynamic import in the factory body. Delete the now-unused
__mocks__/navigatingStore.ts.

Fix path applied: $app/state migration (Markus's recommendation /
Felix's Path 2). See ADR-012.

Refs #553

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:00:44 +02:00
Marcel
89860403f6 fix(notification): remove role=link from view-all button — restores semantically honest button role
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 1m50s
CI / OCR Service Tests (pull_request) Successful in 18s
CI / Backend Unit Tests (pull_request) Successful in 4m12s
CI / fail2ban Regex (pull_request) Successful in 38s
CI / Compose Bucket Idempotency (pull_request) Failing after 10s
CI / Unit & Component Tests (push) Failing after 2m5s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m14s
CI / fail2ban Regex (push) Successful in 39s
CI / Compose Bucket Idempotency (push) Failing after 12s
nightly / deploy-staging (push) Failing after 2m36s
The role=link override on a <button> creates a WCAG 4.1.2 keyboard-contract
mismatch: ARIA role=link tells AT users "press Enter to activate (Space does
nothing)", but the native <button> responds to both Enter and Space. Removes
the override so the element is announced as "button" (accurate).

Test selectors updated from getByRole('link') to getByRole('button')
accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 18:01:38 +02:00
Marcel
6b78557954 refactor(notification-tests): use vi.mocked instead of type cast in call-order test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 17:50:55 +02:00
Marcel
bc2dd3a98a fix(notification): add role=link and touch target to view-all button
Some checks failed
CI / Backend Unit Tests (push) Successful in 4m15s
CI / fail2ban Regex (push) Successful in 39s
CI / Compose Bucket Idempotency (push) Failing after 11s
CI / OCR Service Tests (pull_request) Successful in 17s
CI / Backend Unit Tests (pull_request) Successful in 4m17s
CI / Unit & Component Tests (push) Failing after 1m48s
CI / OCR Service Tests (push) Successful in 17s
CI / Unit & Component Tests (pull_request) Failing after 2m3s
CI / fail2ban Regex (pull_request) Successful in 40s
CI / Compose Bucket Idempotency (pull_request) Failing after 11s
- role="link" restores screen reader link semantics (Leonie blocker)
- min-h-[44px] px-1 meets WCAG 2.2 §2.5.8 and our 44×48px target size
- Comment in handleViewAll explains close-before-navigate ordering
- Tests updated to getByRole('link') + new call-order assertion

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 17:43:11 +02:00
Marcel
8ccc9aba1a fix(notification): replace view-all anchor with button to prevent iframe navigation
SvelteKit's capture-phase link interceptor fires before the component's
onclick handler, so e.preventDefault() was structurally too late to stop
iframe navigation in vitest-browser. Replacing the <a href> with a
<button type="button"> removes the href entirely — the interceptor never
fires — and the existing goto() mock in tests is sufficient.

Also splits the single view-all test into two focused it() blocks and
clears mocks in afterEach to prevent cross-test mock leakage.

Fixes #551

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-12 17:43:11 +02:00