Removes both vi.mock('pdfjs-dist', …) calls that caused the birpc teardown
race (ADR 012). Replaces with static import + makeFakeLibLoader() helper
injected via the libLoader prop on every render() call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two root causes:
1. In-flight test: resolveFetch() was the last line, leaving the async
finally-block writing `training = false` after cleanup destroyed the
component. Awaiting the button becoming re-enabled ensures the finally
block settles before cleanup runs.
2. Success-dismiss test: startTraining() schedules setTimeout(5000) which
fired after cleanup destroyed the component. vi.useFakeTimers() +
vi.runAllTimers() scoped to the describe block drains the timer while
the component is still alive.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Svelte defers DOM updates to microtasks; .query() is a synchronous
snapshot that can fire before the element disappears — making the
absence assertions in AnnotationShape and AnnotationLayer non-deterministic.
Sweeps all 4 instances across both spec files (Sara's ≤5 threshold).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Explicitly states no lint rule is planned; CI guard is the backstop
(addresses Elicit OQ-001 from PR #536 round 4)
- Adds a "when to revisit" note: extract shared DynamicImportLoader<T>
if 3+ components adopt the libLoader pattern
(addresses Markus Keller round-4 observation on PR #536)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a comment above the assertion step so a future developer diagnosing
a birpc-related failure in `npm test` knows where to find the diagnostic.
Addresses Sara Holt + Tobias Wendt round-4 observation on PR #536.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prototype-style assignment was a vi.mock hoisting artifact from the old
version of the file. Rest of the codebase uses class syntax — aligning.
Addresses Felix Brandt round-4 suggestion on PR #536.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds idempotency test: calling init() twice must invoke libLoader only once.
Adds `if (pdfjsReady) return;` guard to satisfy the contract.
Addresses Felix Brandt round-4 suggestion on PR #536.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
-F (fixed string) matches the literal pattern [birpc] rpc is closed
without relying on BRE bracket escaping, making the intent explicit
and immune to accidental regex interpretation.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The birpc guard step writes to /tmp/coverage-test-<run_id>.log and exits 1
when a race is detected. Without this file in the artifact, the evidence
disappears when the runner tears down — only the exit code remained visible.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
.catch(()=>{}) swallowed the rejection, so the test passed vacuously even
if a future refactor silently caught the error. rejects.toThrow() proves
the propagation contract holds before asserting pdfjsReady stays false.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add explicit set -eo pipefail so npm test:coverage exit code
propagates through the pipe (not just tee's always-0 exit)
- Scope log file to github.run_id to prevent stale-log false positives
on retried steps sharing the same runner /tmp
- Tighten grep pattern to \[birpc\] rpc is closed to avoid matching
unrelated log lines that happen to contain "rpc is closed"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Regression-protection test: init() propagates the loader rejection
before pdfjsReady is set, so the renderer stays in a safe unready state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without untrack, a reactive libLoader prop reference change would
reinitialise the whole renderer and lose all loaded state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Exporting LibLoader gives the type a stable, named identity.
PdfViewer.svelte and PdfViewer.svelte.spec.ts now import it directly
instead of using Parameters<typeof createPdfRenderer>[0].
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- removes unreachable `; exit ${PIPESTATUS[0]}` — already covered by pipefail (Tobias)
- adds explicit `shell: bash` to both new steps for clarity (Tobias)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- import PdfViewer left mid-file from vi.mock hoisting — no longer needed (Sara/Felix)
- adds one-line comment explaining as unknown as cast is an intentional partial fake (Felix)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents why vi.mock(module, factory) races with birpc teardown for
dynamically-imported modules, the libLoader injection pattern used to fix
#535, and the residual exceptions ($app/*, $env/*) that are safe to keep
as vi.mock because they are resolved statically before any test runs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Captures npm run test:coverage output with tee and adds an always-run step
that greps for the teardown-race fingerprint. Any future regression where a
vi.mock factory races with birpc teardown will now surface as an explicit CI
failure rather than a silent exit-1 after all tests report green (#535).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes both vi.mock('pdfjs-dist', factory) and
vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', factory) from
PdfViewer.svelte.spec.ts — the ManualMockedModule registrations that were
racing with vitest-browser-playwright's birpc teardown channel.
PdfViewer.svelte now accepts an optional libLoader prop (typed as
Parameters<typeof createPdfRenderer>[0]) that is passed untracked to
createPdfRenderer(). Tests supply a vi.fn() fake loader directly as a prop;
production code uses the default loader that imports the real pdfjs-dist.
The birpc route handler for pdfjs-dist is never registered, so no teardown
race is possible. Fixes#535.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an optional LibLoader parameter (defaults to the real pdfjs-dist dynamic
imports) and a failing test that verified the loader is called during init().
This is the first step toward removing ManualMockedModule registrations that
race with vitest-browser-playwright's birpc teardown (#535).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`ip route` (iproute2) is not installed in the Gitea runner container,
causing the smoke test step to exit 127. /proc/net/route is a kernel
virtual file that is always present on Linux; awk decodes the
little-endian hex gateway field to dotted-decimal without any external
binary dependency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unquoted variable expansion is safe here since the value contains
no spaces or glob characters, but quoting is the correct default
and keeps the script consistent with surrounding style.
Addresses review suggestion by Felix Brandt and Tobias Wendt.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
If `ip route show default` returns no output the old code passed
an empty string to curl --resolve, producing a confusing error 6
("couldn't resolve host") with no indication that gateway detection
had failed. The new guard exits immediately with a clear message.
Addresses review concern raised by Tobias Wendt.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Job containers run in bridge network mode (runner-config.yaml). Inside
a bridge-networked container 127.0.0.1 is the container's own loopback;
Caddy on the host is unreachable there, causing an immediate ECONNREFUSED.
Use the Docker bridge gateway IP instead — the host's docker0 interface
where Caddy (bound on 0.0.0.0:443) is reachable from the container.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a packageRule matching .gitea/workflows/** digest updates with
automerge: false. Digest bumps for images running --privileged --pid=host
have root-equivalent host access and must not be auto-merged.
Addresses Nora's review concern on #537.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the three failure modes Sara flagged: Caddy stopped (explicit
systemctl error), symlink missing/mis-pointed (silent reload, stale
smoke test), and Docker socket / nsenter unavailable (container error).
Each failure mode includes symptoms and recovery steps.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the stale generic runner provisioning docs with an accurate
description of the actual two-container setup on the Hetzner VPS.
Document the nsenter pattern for running host-level commands (systemctl)
from containerised CI steps, and the Caddyfile symlink contract that the
reload step depends on.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same gap as nightly.yml: production deploys also need Caddy to reload
the updated Caddyfile before the smoke test validates the public surface.
Uses the same nsenter pattern introduced in the previous commit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`sudo systemctl reload caddy` does not work from inside a DooD job
container: `systemctl` is absent from Ubuntu container images and
container processes cannot reach the host systemd without entering its
namespaces. Replace with `docker run --privileged --pid=host ubuntu:22.04
nsenter -t 1 -m -u -n -p -i -- /bin/systemctl reload caddy`, which uses
the already-mounted Docker socket to spin up a privileged sibling
container that enters the host PID namespace via nsenter. Tested live on
the Hetzner VPS. No sudoers entry required.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a `sudo systemctl reload caddy` step between the docker compose
deploy and the smoke test. This ensures any committed Caddyfile changes
are applied before the public surface is verified.
Previously the workflow had no mechanism to push Caddyfile changes to
the running host daemon. A Caddyfile edit would land in the repo but
Caddy would keep serving the previous config, causing the smoke test to
catch a stale header or still-proxied /actuator route rather than the
intended current config.
This step also surfaces the root cause of today's port-443 failure
explicitly: if Caddy is not running, the step fails with a clear service
error rather than a misleading "Failed to connect to port 443" from curl.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
