Skip to content

Fix cross-platform chrome/safari test expectations; stabilize frontend test runtime - #1

Merged
t41372 merged 1 commit into
mainfrom
codex/fix-ci-issues-and-clean-codebase
Apr 5, 2026
Merged

Fix cross-platform chrome/safari test expectations; stabilize frontend test runtime#1
t41372 merged 1 commit into
mainfrom
codex/fix-ci-issues-and-clean-codebase

Conversation

@t41372

@t41372 t41372 commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Motivation

  • CI was failing due to macOS-only path assumptions in browser discovery tests and frontend test flakiness from filesystem watch limits and short timeouts.

Description

  • Update vault-core tests to assert platform-specific Chrome user-data and Safari defaults and include Flatpak Chromium paths for Linux (edited src-tauri/crates/vault-core/src/chrome.rs).
  • Increase Vitest timeouts by setting testTimeout and hookTimeout to 15_000 in vitest.config.ts to reduce coverage-mode and slow-runner timeouts.
  • Add a Vite dev-server watch.ignored entry for src-tauri/target/** in vite.config.ts to prevent ENOSPC file-watcher failures during Playwright/e2e runs.
  • Keep code formatting and lint rules enforced so Rust fmt/clippy remain satisfied.

Testing

  • Ran bun install --frozen-lockfile and bun run check:js, and frontend unit tests and bun run test:e2e completed successfully.
  • Ran Rust checks cargo fmt --all --check, cargo clippy --workspace --all-targets --all-features -- -D warnings, and cargo test --workspace --all-targets, and they all passed.
  • Ran supply-chain checks bun run check:supply-chain and coverage tasks bun run coverage:js and bun run coverage:rust, and both JS and Rust coverage verification passed (JS 100%, Rust 100%).
  • Ran JS mutation testing bun run mutation:js (Stryker) which completed with a mutation score of 86.88% (above the threshold), and started the Rust mutation runner (cargo-mutants) where baseline completed but the full run is lengthy and was not fully completed in this iteration due to runtime constraints.

Codex Task

@t41372
t41372 merged commit d0f80d1 into main Apr 5, 2026
10 checks passed
@t41372
t41372 deleted the codex/fix-ci-issues-and-clean-codebase branch April 11, 2026 02:14
t41372 added a commit that referenced this pull request May 22, 2026
… i18n

Why
- Codex review flagged three Blocking/High issues against feat/v0.3-redesign-2:
  (1) the ⌘K palette in shell.tsx sent `{ search, limit, offset }` to
      query_history and tried to read `response.rows`, but the real
      contract is `{ q, page, cursor, ... }` returning `items`. Result:
      palette searches were silently empty on desktop, and the existing
      tests had been mocking the wrong shape.
  (2) Paper UI still leaked raw English copy (char/chars counter, "Remove
      tag {tag}" aria, "Calendar" dialog label, "now"/"first" year-rail
      footers, dashboard greetings). i18n is a shipping contract, not
      polish.
  (3) Theme was held twice — once by the shell as a private useState,
      once by Settings → Appearance via applyPaperPreferences. Toggling
      one didn't update the other, so the buttons drifted.

What
- shell.tsx: palette now calls `backend.queryHistory({ q, limit, sort })`
  and maps `response.items` with the real HistoryEntry shape (id/url/
  domain/title/visitedAt/visitTime). Removed the orphan PaletteRow
  interface. shell.test.tsx mocks `backend.queryHistory` with valid
  HistoryQueryResponse fixtures instead of the legacy rows shape.
- i18n: added 3-language entries for the paper detail panel's char
  counter (singular + plural template), the remove-tag aria label, the
  calendar dialog aria, the year-rail now/first footer captions, and
  the dashboard morning/afternoon/evening greetings. The dashboard's
  hand-rolled greeting map is gone — it now reads
  `dashboard.greetingMorning/Afternoon/Evening` like every other copy
  in that route. PaperDetailPanelCopy + PaperCalendarPopoverCopy +
  PaperYearRailProps + paper-view.tsx copy interface all gained the
  new keys; buildPaperDetailPanelCopy + buildPaperContactSheetCopy
  thread them through. i18n parity 100% (2798 keys × 3 locales).
- paper-preferences.ts: applyPaperPreferences now dispatches a
  `pathkeep.paperPreferencesChanged` CustomEvent (with the resolved
  preferences in `detail.preferences`) after applying + persisting.
  shell.tsx subscribes to that event so the topbar theme button stays
  in sync with Settings → Appearance toggles; settings/appearance-
  section.tsx subscribes too so flipping theme via the shell button
  updates the radio without re-mount. shell.tsx's handleToggleTheme
  now routes through applyPaperPreferences instead of mutating a
  private useState — single source of truth, one persist call.

Context
- Codex findings #1, #5, #6. Findings #2 (paper pagination), #3
  (og:image fetch trigger), #4 (coverage gate restoration) tracked
  separately and remain pending in BACKLOG.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
t41372 added a commit that referenced this pull request May 27, 2026
Why
  Claude review findings #1–4: four dispatch arms in
  `dev_ipc_bridge/dispatch.rs` (`test_ai_provider_connection`,
  `run_ai_queue_jobs`, `search_ai_history`, `ask_ai_assistant`) call
  worker code that builds its own short-lived `tokio::runtime::Runtime`
  via `Runtime::new()` and calls `block_on(...)` on a future. Dropping
  that runtime inside another async runtime's current-thread context
  panics with "Cannot drop a runtime in a context where blocking is
  not allowed."

  The pattern is identical to the `run_backup_now` panic that f580761
  fixed: production Tauri commands sidestep it because
  `run_blocking_command` wraps every synchronous worker call in
  `tauri::async_runtime::spawn_blocking`. The dev-IPC bridge dispatcher
  runs directly on the axum server's tokio thread, so the panic blows
  up the bridge thread and the client sees `socket hang up` with no
  body.

  AI is currently `[!blocked]` per BACKLOG, so this is dormant on the
  v0.3 ship. But the moment `WORK-AI-V03-A` lands and someone exercises
  any of these arms through the dev-IPC bridge (browser-preview e2e,
  Playwright automation, Vercel-hosted preview), it'll panic exactly
  like `run_backup_now` did. Cheap to fix while the pattern is fresh.

What
  - Each of the four AI dispatch arms wraps its
    `worker_bridge::*_impl(...)` call in
    `tokio::task::spawn_blocking(move || ...).await`, mirroring the
    existing `trigger_og_image_refetch` arm at line 165 and the
    `backup_now_off_thread` helper added in f580761.
  - `JoinError` from the blocking task flattens into the
    `Result<_, String>` error channel with an arm-named context message
    so a future panic on the inner work surfaces as a clean error
    instead of a generic transport failure.
  - Inline comments document the shared root cause and reference the
    Claude review findings so the next reader recognises the pattern.

How
  Inline `spawn_blocking` per arm (matching the existing pattern at
  line 165) was chosen over a generic helper because the four arms
  have different request/response types and each arm-specific error
  message is more useful in logs than a generic
  "ai_off_thread join failed."

  Other AI-shaped arms (`build_ai_index`, `replay_ai_job`,
  `cancel_ai_job`, `load_ai_queue_status`, `load_ai_assistant_job`)
  were traced and found to NOT reach `block_on` — they either only
  enqueue + spawn a background OS thread via `maybe_spawn_ai_queue_drain`
  or stay entirely synchronous. Not wrapping them avoids unnecessary
  thread-pool churn for the common no-op paths.

  The existing dispatch-coverage test helper `dispatch_for_coverage`
  uses `catch_unwind` defensively. After this fix the panic no longer
  fires for these arms; the helper stays as belt-and-suspenders
  protection for any future code paths that introduce a similar
  Runtime::new() shape before they're spotted and wrapped.

Verification
  - `cargo build` clean.
  - `cargo test --features devtools-bridge` — all 4 dispatch tests
    pass (`dispatch_command_decodes_all_browser_mirror_command_payloads`
    walks the full coverage matrix including these 4 arms).
t41372 added a commit that referenced this pull request Jul 5, 2026
… step

check:coverage now passes, so CI reached test:e2e and revealed two stacked
browser-preview failures (masked until now):

1. Product bug — the archive unlock gate covered onboarding. `showUnlockGate`
   fired on `encrypted && !unlocked` without requiring `initialized`, so a fresh
   archive's onboarding snapshot (initialized:false, encrypted:true, unlocked:false)
   floated the blocking unlock modal over the Dashboard/onboarding and its backdrop
   swallowed the "Start setup" click. You can't unlock an archive that doesn't exist
   yet. Add the `initialized` clause: a real initialized+encrypted+locked archive
   still shows the gate (at-rest protection unchanged), but onboarding no longer is.
   Covered both directions in shell.test.tsx.

2. Stale test — f943be8 inserted an off-by-default AI opt-in step between Schedule
   and Ready but never updated shell.spec.ts, so after fix #1 the walk landed on the
   AI step and timed out. Click through onboarding-ai-skip (AI stays off, the default).

Verified in a Linux-matching run: test:e2e 4/4, check:js, coverage:js (shell.tsx 100%),
and check:mutation (100 score) all green. NOTE: test:e2e:desktop-bridge:truth still has
one further pre-existing failure behind this (a fullRebuild core-intelligence flow
reporting processedVisits:0) — tracked separately; it needs a Rust/backend look and
was never reached by CI before this commit.

Claude-Session: https://claude.ai/code/session_01M18MvvFGLQzLVyhxvK5WMy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant