docs: test-suite gap audit — drops, overfits, missing-coverage backlog, CI re-check (#4489)

plan.md

@@ -0,0 +1,443 @@
+# Test Suite Audit & Improvement Plan
+
+Multi-agent audit of the OpenHuman test surface (2,367 files / ~25,900 test declarations per
+`docs/test-inventory/REPORT.md`). Ten parallel auditors each read their slice of the inventory
+**plus the actual test source**; every "drop this test" / "this is overfitted" recommendation was
+then adversarially reviewed by a second skeptic pass that tried to refute it by re-opening the
+cited files (40 verdicts: 32 confirmed, 8 refuted — the refuted ones are documented below so
+nobody deletes them later). A final completeness critic audited the audit itself.
+
+**Headline findings**
+
+1. The suite is **broadly healthy** — security/policy adversarial tests, tunnel crypto, composio
+   client, core_process tests are genuinely behavioral and strong.
+2. The two **highest-risk untested surfaces in the codebase**:
+   `src/openhuman/security/policy/command_checks.rs` + `path_checks.rs` (the documented
+   autonomy-enforcement core: `classify_command`, `gate_decision`, `is_workspace_internal_path`)
+   and `src/openhuman/encryption/core.rs` (Argon2id + AES-256-GCM primitives) have **zero unit
+   tests**.
+3. A meaningful chunk of the suite **never runs in CI**: the mock server's own socket-auth tests,
+   most of `scripts/__tests__/`, and the Pester Windows-install test are invoked by no workflow.
+   A never-run test is worse than no test — it reads as coverage.
+4. The 80% changed-lines coverage gate has a **scope hole**: `scripts/**` (including the entire
+   shared mock backend every other suite depends on) is outside all path filters.
+5. ~130 test declarations are **safe to delete** (verified duplicates, typeof-only boilerplate,
+   empty compile-only tests, copy-paste connector templates) — see §2.
+
+---
+
+## 1. Method
+
+- **Audit fan-out (Sonnet):** frontend unit (components/pages; services/store/hooks/lib),
+  frontend E2E (WDIO + Playwright), Rust unit (agent/memory; channels/providers/platform;
+  security/config/infra), Rust integration/E2E, Tauri shell, the mock/harness infrastructure
+  itself, and a cross-cutting gap hunter comparing product domains ↔ inventory.
+- **Skeptic verify (Opus, high effort):** every drop/overfit finding re-verified against source;
+  refuted items retained with the refutation recorded.
+- **Completeness critic (Opus):** audited the audit for missing dimensions (mutation testing,
+  fuzzing, a11y, perf, flake data, CI orphans…).
+
+Legend: ✅ = skeptic-confirmed · ⚠️ = plausible, not independently re-verified · ❌ = refuted (do **not** act).
+
+---
+
+## 2. Tests to DROP
+
+### 2.1 Confirmed safe to delete
+
+| # | File | What | Why |
+|---|------|------|-----|
+| ✅ | `app/src/components/settings/hooks/__tests__/useSettingsNavigation.test.tsx` | whole file | Asserts a retired, hardcoded-empty `breadcrumbs` field across 8 routes — tests a constant. Route resolution is fully covered by the sibling `useSettingsNavigation.coverage.test.tsx`. |
+| ✅ | `app/src/services/api/{graphCentralityApi,graphCohesionApi,memoryFreshnessApi,connectionPathApi,memoryTimelineApi,entityAssociationsApi,namespaceOverviewApi}.test.ts` | the copy-pasted `exposes the public surface` test in each (7 files) | typeof-only assertions on aggregate objects **no consumer imports** (tabs import the named exports directly); the functions are behaviorally tested above in each file. Do one grep-and-delete pass. |
+| ✅ | `src/openhuman/agent/harness/harness_gap_tests.rs` | `datetime_section_is_static_grounding_rule_not_a_volatile_timestamp` | Strict subset of `agent/prompts/mod_tests.rs::datetime_section_is_static_grounding_rule_without_volatile_timestamp`; the file's own header lists item 6 as covered elsewhere. |
+| ✅ | `app/src-tauri/src/lib_tests.rs` | `setup_tray_function_signature_compiles`, `tray_setup_logging_patterns_exist`, `app_runtime_type_exists`, `is_daemon_mode_detects_daemon_flag` | Empty bodies / comments-only / the one real assertion is commented out / result discarded with `let _`. Cannot fail; verify nothing. |
+| ✅ | `app/src-tauri/src/deep_link_ipc.rs` | `extract_deep_link_urls_filters_correctly` | Re-implements the prefix filter inline instead of calling the production `extract_deep_link_urls()` — passes even if the real filter regresses. Better fix: refactor to take an args slice like the Windows sibling (`collect_deep_link_urls_from_args`), then test the real function. |
+| ✅ | `src/openhuman/routing/factory.rs` | `factory_constructs_without_panic_when_runtime_enabled`, `factory_llamacpp_provider_constructs_without_panic`, `factory_custom_openai_provider_constructs_without_panic`, `factory_lm_studio_provider_constructs_without_panic` | Skeptic traced the whole construction path — provably infallible (pure struct init), so the tests cannot fail. One test's comment claims to verify probe-URL selection but the body asserts nothing; fields are private so strengthening is blocked. |
+| ⚠️ | `app/src/components/chat/ArtifactCard.test.tsx` | whole file (281 lines) | Duplicates `__tests__/ArtifactCard.test.tsx` test-for-test (in-progress label, download→done flow, error truncation + Show more, Retry). Keep the `__tests__/` version. |
+| ⚠️ | `app/src/components/settings/panels/RecoveryPhrasePanel.test.tsx` | whole file (1 test) | Subsumed by the 35-test `__tests__/RecoveryPhrasePanel.test.tsx`. |
+| ⚠️ | `src/openhuman/threads/ops_tests.rs` | `sanitize_generated_title_*`, `collapse_whitespace_*`, `title_log_fingerprint_*`, `title_from_user_message_*` | `threads/title.rs` inline tests already cover the same functions with equivalent cases. Keep the copies in `title.rs` (the owning module). |
+| ⚠️ | `src/openhuman/channels/providers/whatsapp_tests.rs` | 7 × `whatsapp_parse_<type>_message_skipped` | All hit the identical `type != "text" → continue` branch; collapse to one parameterized loop over the type strings. |
+| ⚠️ | `src/openhuman/routing/telemetry.rs` | `emit_does_not_panic` ×3 variants | Fire-and-forget calls with no assertion. |
+| ⚠️ | `app/src-tauri/src/deep_link_ipc.rs` | `no_primary_returns_appropriate_result` | Its own comment admits it can't reach the production branch; asserts stdlib `UnixStream::connect` errors on a bogus path. |
+| ⚠️ | `app/test/e2e/specs/smoke.spec.ts` | the permanently-`it.skip`ped auth-deep-link test | Skipped for a documented flake with no tracking issue/owner. File an issue and fix, or delete. |
+
+### 2.2 Consolidate, don't delete: the connector template family
+
+`connector-{airtable,asana,clickup,confluence,google-calendar,google-drive,google-sheets,notion,slack-composio,todoist,youtube}.spec.ts`
+— **11 WDIO files, ~99 test declarations, byte-identical** except for toolkit name/slug strings.
+Playwright already proves the fix: `connector-session-guard-matrix.spec.ts` parametrizes the same
+assertions over a `TOOLKITS` array in one file. Extract a `runConnectorContract(toolkit, opts)`
+helper, keep **`connector-jira.spec.ts`** (subdomain-field UI) and
+**`connector-gmail-composio.spec.ts`** (400-on-fetch-emails handling) as bespoke specs.
+Net: ~100 declarations → ~10 with identical per-toolkit coverage.
+
+### 2.3 ❌ Refuted — keep these (recorded so they don't get re-proposed)
+
+| File | What | Why it stays |
+|------|------|--------------|
+| `src/openhuman/memory/schema_tests.rs` | registry-sync + unknown-fn tests | **False duplicate.** `memory/schema/` (singular, `memory_tree` namespace) and `memory/schemas/` (plural, `memory` namespace) are two distinct live registries with disjoint function sets. These are the *only* parity/unknown-fn guards for the `memory_tree` controller surface. |
+| `src/openhuman/channels/providers/qq_tests.rs` | `test_name` | Sole coverage of `QQChannel::name()`, which keys routing (`routes.rs:345`) and the channel map (`runtime/startup.rs:701`). A rename would ship uncaught. |
+| `src/openhuman/provider_surfaces/schemas.rs` | `all_schemas_returns_two` etc. | Weak but the only guard that the registration lists are populated. **Improve** (see §3), don't delete. |
+| `src/core/jsonrpc_tests.rs` | wallet-message drift guard (L1421) | The literal pin is load-bearing: three Rust producers and six frontend components hardcode the same string; the pin is what forces a human to update the desynced sites on a wording change. |
+| `tests/x402_twit_sh_live.rs` | `#[ignore]`d live x402 test | Only coverage artifact for the money-moving `X402RequestTool` (compile-pins its API + manual E2E harness). Costs nothing in CI. |
+| `app/test/e2e/specs/gmail-flow.spec.ts` | blanket drop of 8 tests | Over-reach: 4 of the 8 carry a real "app still boots with revoked/expired-token mock state" guard nothing else reproduces. Only 9.1.2 and 9.3.3 are true no-ops. Fix the fixture instead (§3). |
+
+---
+
+## 3. OVERFITTED tests to rewrite
+
+| # | File / test | Problem | Fix |
+|---|-------------|---------|-----|
+| ✅ | `src/openhuman/agent/prompts/mod_tests.rs::grounding_contract_requires_exact_numeric_evidence` | Pins 5 verbatim prose substrings of the grounding contract — breaks on any copywriting pass. | Behavioral guarantee ("contract appended on every build path") already covered by the marker-based test; convert this to a single explicitly-labeled wording-lock, or assert stable structural markers. |
+| ✅ | `src/openhuman/agent/prompts/mod_tests.rs::identity_section_creates_missing_workspace_files` | Also string-matches SOUL.md brand-voice prose (`"Don't validate FUD"`). | Split: (a) files created + seeded from the checked-in template (compare against template file content); (b) a narrow, labeled brand-voice lock if the phrase must stay pinned. |
+| ✅ | `app/src-tauri/src/core_process_tests.rs::startup_timeout_cleanup_aborts_task_and_clears_slot` | 5 substring checks against one human-readable diagnostic string. **Caveat:** it also asserts real behavior (task slot cleared, shutdown token cancelled) — preserve those two assertions. | Return a small struct (attempt, port, ready_signal, port_open, task_state) + display string; assert struct fields, one loose `contains` on the text. |
+| ✅ | `src/openhuman/hooks/../useDaemonLifecycle.test.ts` (`app/src/hooks/__tests__/`) | Pins exact `console.log` strings as an effect-rerun proxy. Listener-count assertions are legit — keep them. | Drop the log-text pinning; keep listener/startDaemon observable assertions. |
+| ✅ | `src/openhuman/provider_surfaces/schemas.rs::all_schemas_returns_two` / `all_controllers_returns_two` | Magic-number count breaks on any legitimate 3rd controller. | Replace with `schemas().len() == controllers().len()` parity + presence of a known op (`list_queue`). Standardize this as a shared `assert_schema_controller_parity()` helper — the `== N` pattern repeats across ~15 domains. |
+| ✅ | every `connector-*.spec.ts::composio_sync RPC routes to mock backend` | Name promises routing; body only asserts the session didn't crash (original assertion removed per inline comment). | Rename to what it checks, or move the real routing assertion to a native-provider connector where sync actually hits the mock. |
+| ✅ | `tests/composio_raw_coverage_e2e.rs::composio_controller_schema_catalog_covers_all_declared_functions` | Hardcoded `inputs.len() == N` per function breaks on additive optional params. | Assert on specific input *names*, keep the meaningful namespace/name/output checks. |
+| ⚠️ | `app/src/components/chat/__tests__/ApprovalRequestCard.test.tsx` opaque-surface test | Asserts exact Tailwind class names + a regex banning opacity utilities. | Assert computed style, or accept it as a labeled visual-regression lock. |
+| ⚠️ | `AppWalkthrough.test.tsx` | `querySelector('div.bg-gradient-to-r')` and hardcoded `returns 13 steps`. | `data-testid` for the progress bar; drop the step count (first/last-target tests carry the signal). |
+| ⚠️ | `app/test/e2e/specs/gmail-flow.spec.ts` | "if UI not found, log and pass" pattern — degrades to no-op instead of failing. | **Fix the fixture:** seed deterministic Gmail-skill discovery in the mock, then assert unconditionally with no branching. |
+
+---
+
+## 4. MISSING coverage — prioritized backlog
+
+### P0 — security, crypto, data-loss, ingress (do these first)
+
+1. **`security/policy/command_checks.rs` + `path_checks.rs` — ZERO tests.** The enforcement core
+   of the entire autonomy model. Build a parametrized gate-matrix suite:
+   (tier `readonly/supervised/full`) × (`CommandClass` Read/Write/Network/Install/Destructive) ×
+   path-root, asserting `gate_decision` Allow/Prompt/Block; `is_always_forbidden` blocks
+   system/credential dirs even when tier+trusted_roots would allow; `is_workspace_internal_path`
+   fail-closed regardless of tier; `classify_command` "unrecognized ⇒ Write" fail-safe against
+   pipes/redirects/chained commands. (Note: `policy_tests.rs` — 2,606 lines — is excellent but
+   covers the *higher-level* policy, not these modules.)
+2. **`encryption/core.rs` — ZERO tests** on the Argon2id/AES-256-GCM primitives. Round-trip;
+   wrong password / tampered ciphertext / tampered nonce rejection; fresh salt+nonce per call;
+   KDF determinism. Plus one RPC→crypto→RPC integration test (schema tests currently never touch
+   real ciphertext).
+3. **`memory/read_rpc/admin.rs::delete_source_rpc` — ZERO tests** on a 427-line destructive
+   cascade-delete (chunks, embeddings, entity-index, content files, orphan-tree). Cover: exact
+   source_id scoping (prefix siblings untouched); shared path_scope/collection trees NOT torn down
+   while referenced; orphan cascade when fully orphaned; idempotency; legacy partial-delete
+   recovery.
+4. **Event bus panic isolation** (`src/core/event_bus/bus.rs`): production `catch_unwind` exists
+   precisely so one handler can't kill the loop — no test exercises it. Two subscribers, one
+   panics, assert both keep receiving subsequent events.
+5. **Webhook ingress flood** (`webhooks/router.rs`): externally-triggered, unauthenticated-by-default
+   surface with no rate-limit/backpressure test. If no such logic exists, that's a **product** gap.
+6. **E2E: core-process crash/recovery.** Blocked on a missing debug-only `stop_core_process`
+   Tauri command (documented in `connectivity-state-differentiation.spec.ts`'s own header). Add the
+   command, then: kill core mid-chat-turn → recoverable error (not a hang) → offline state →
+   `restart_core_process` restores a working session.
+7. **E2E: RPC bearer auth failure.** No spec covers `core_rpc_relay` with a missing/tampered/expired
+   token; `tauri-commands.spec.ts` is happy-path only. Also add the in-memory token-handoff
+   round-trip test (`core_rpc_token` → relay → Authorization header → 401 without it).
+8. **CI wiring (see §5):** un-run harness self-tests are a P0 process gap.
+
+### P1 — core workflows
+
+- **Approval gate × agent turn**: harness-level test that a Write/Destructive-class turn parks
+  pending approval while background/cron turns bypass, plus the **concurrent double-decide race**
+  on one request_id (exactly-one-terminal-outcome). Frontend twin: composer blocked while an
+  approval card is pending on the thread (`composerInteractionBlocked` wiring is untested).
+- **`TransportManager.raceLanAndTunnel()`**: the LAN-vs-tunnel race is entirely untested despite
+  the docstring claiming otherwise — LAN wins (tunnel closed), tunnel wins (LAN closed), both
+  fail, win-then-fail → `reset()` re-race.
+- **Memory `path_scope` invariant**: two source_ids sharing a path_scope must summarize into one
+  collection tree (the documented dedupe-key-vs-scope contract).
+- **Hostile webhook payloads** (whatsapp/lark/linq/dingtalk/mattermost): wrong JSON types on
+  expected fields, deep nesting, oversized bodies — current "malformed" tests only cover missing
+  fields.
+- **Socket reconnect/backoff state machine**: only yuanbao has explicit backoff-schedule tests;
+  mirror them on the primary socket client (increase → cap → reset-on-success), plus
+  socketService's reaction to mid-session auth expiry (teardown + fresh token on reconnect).
+- **web3 swap/bridge/dapp tool wiring** (money-moving, untested at tool layer): schema rejection,
+  delegation to ops with stubbed quote, error propagation. Plus one cross-domain quote→execute
+  JSON-RPC E2E against a stubbed wallet.
+- **`threadSlice.ts` (501 lines) and `accountsSlice.ts`**: no test files at all.
+- **`CoreProcessHandle::restart()`** and **`core_rpc.rs::apply_auth()`** error paths (Tauri side).
+- **E2E journey spec**: onboarding → connect channel → chat turn using it → approval gate →
+  tool result renders → new thread → memory recall of the earlier interaction. Individual pieces
+  exist; no single continuous-session spec does.
+- **Approval-gate Playwright mirror**: `agent-harness-behaviors.spec.ts` exists only in the slower
+  WDIO lane.
+- **AgentAccessPanel tier cross-check**: which sub-controls are enabled/hidden per autonomy tier.
+- **Onboarding credential forms**: happy-path persistence + invalid-input-blocks-navigation for
+  CustomInference/CustomSearch pages.
+- **Coverage-gate scope**: add `scripts/**` (and `packages/**`) to the path filters (§5).
+- **~20 RPC controller domains with zero E2E references** (`recall_calendar`, `tinyplace`,
+  `devices`, `people`, `redirect_links`, …) — one RPC round-trip each; full list in Appendix A.3.
+
+### P2 — polish / lower blast radius
+
+- Unicode-homoglyph spoofing cases through `is_command_allowed` / path prefix checks (current
+  multibyte tests are panic-safety only).
+- Tunnel `framing.ts` adversarial reassembly (out-of-order, duplicate seq, truncated final chunk).
+- Composio second-consecutive-auth-error must surface, not loop.
+- `socketSlice` / `channelConnectionsSlice` / `pttSlice` / `providerSurfaceSlice` reducer tests;
+  `Invites.tsx` page test.
+- `redact_url_for_log` direct unit test (token-leak prevention).
+- Port native-free WDIO-only specs (memory-sync-schedule, skill-activation-persistence,
+  chat-thread-todo-strip, chat-background-activity-panel) to Playwright; maintain a documented
+  allowlist of intentionally native-only specs.
+- Symlink-chain (depth >1) cases for `is_workspace_internal_path` / `validate_path_within_root`.
+- `tauri-plugin-ptt` Rust command surface (iOS is non-shipping — lowest priority).
+
+---
+
+## 5. Harness, mocks & CI — audit results
+
+**Good news:** the three mock entry points (`scripts/mock-api-core.mjs`, `scripts/mock-api-server.mjs`,
+`app/test/e2e/mock-server.ts`) are thin shims over **one** implementation in `scripts/mock-api/`,
+so drift risk is ~nil. Deterministic seeding (`fuzzyNumber`/`fuzzyPick` off `mockBehavior.seed`)
+already exists. `httpFaultRules` gives generic per-route status/body/latency injection.
+
+**Holes found (verified by exhaustive grep of package.json + workflows):**
+
+1. **Orphaned tests — never run in any CI job:** `scripts/mock-api/socket.auth.test.mjs`,
+   `socket.transport.test.mjs`, most of `scripts/__tests__/*.test.mjs`, and the Pester
+   Windows-install test (`OpenHumanWindowsInstall.Tests.ps1`). The mock server's socket-auth
+   behavior — which every E2E suite depends on — is itself unverified in CI.
+   → Add `pnpm test:scripts` (`node --test scripts/**/*.test.mjs`) + a CI step; wire the Pester lane.
+2. **Coverage-gate scope hole:** the diff-cover gate only arms on `frontend || rust-core || rust-tauri`
+   path filters; `scripts/**` (mock backend, debug runners, coverage checkers) is outside all of
+   them. A mock-backend rewrite ships with zero coverage pressure.
+   → Add `scripts/mock-api/**` + `scripts/*.mjs` to the filters; optionally feed a c8 lcov into the
+   same diff-cover invocation.
+3. **Fault-injection ceiling:** the mock can inject clean HTTP errors and latency, but **cannot**
+   simulate connection reset mid-response, hung requests, truncated chunked bodies, or malformed
+   (non-JSON 200) responses — all real outage shapes.
+   → Add `mode: "reset"` and `mode: "malformed"` to `httpFaultRules`; document a small "chaos
+   toolkit" (500 storm, slow drip, auth-expiry-mid-session) so authors stop adding bespoke
+   one-off behavior flags (auth.mjs already has ~6 that `httpFaultRules` could express).
+4. **Order-dependent E2E flake source:** `wdio.conf.ts` runs all specs in ONE session with state
+   deliberately carried spec-to-spec, over a mock with module-level mutable state that only resets
+   when a spec remembers to call `/__admin/reset`. A spec failing before its reset poisons the next.
+   → Unconditional `/__admin/reset` in a WDIO `afterTest`/per-spec hook.
+5. **Mock↔backend contract drift:** nothing validates mock route responses against the TS types the
+   frontend api clients expect. → zod/JSON-schema contract test over a sample of mock responses;
+   extend the existing `rpcMethods.test.ts` drift-guard pattern (frontend constants ↔ Rust schema
+   source) to tool names and event names.
+
+---
+
+## 5bis. Re-check under the two-lane CI (2026-07-03, after #4486)
+
+CI moved to a two-branch model: PRs → `main` run a fast lane (`pr-ci.yml`: quality checks +
+unit tests **only for changed files** — `vitest related` / domain-scoped `cargo llvm-cov`), and a
+standing main→release PR (auto-refreshed by `prepare-release-pr.yml` on every push to main) runs
+the full lane (`release-ci.yml`: full unit suites, Rust mock-backend E2E, Playwright, desktop E2E
+on 3 OSes, gated by "Release CI Gate"). Re-audit of every CI-related finding:
+
+### Improved by the change
+
+- **Full-suite cadence is now structural**: because the standing release PR synchronizes on every
+  main push, the complete unit + Rust E2E + desktop matrix runs against every mainline state —
+  cross-domain breakage can no longer reach a *release* unseen. (It can still land on `main`
+  unseen; see below.)
+- Legacy `e2e.yml` / `e2e-playwright.yml` / `test.yml` are now `workflow_dispatch`-only — the
+  fan-out lives behind the release gate instead of ad-hoc triggers.
+
+### Findings from §5 that are STILL open (re-verified by grep on the new workflows)
+
+| Plan item | Status |
+|---|---|
+| Orphaned harness self-tests (`scripts/mock-api/socket.{auth,transport}.test.mjs`, most of `scripts/__tests__/`, Pester `test:install-ps1`) | **Still orphaned.** No workflow references them in either lane (`pnpm docs:test` remains the only `scripts/__tests__` entry point; no `pwsh` anywhere). |
+| Coverage-gate scope hole for `scripts/**` | **Still open, and slightly worse.** The new `pr-ci.yml` path filters still exclude `scripts/mock-api/**` and `scripts/*.mjs` (only `ci-cancel-aware.sh`, `test-rust-e2e.sh`, `test-rust-with-mock.sh`, `scripts/ci/*.sh` appear). A mock-backend rewrite now triggers **zero tests of any kind on the PR lane** — the code it can break (Rust E2E, Playwright, desktop E2E) only runs at the release PR. |
+| Per-spec mock `/__admin/reset` WDIO hook | **Not landed** (`wdio.conf.ts` unchanged). Matters more now: the full WDIO matrix is the release gate, so its order-dependent flakes directly block releases. |
+| Test-file→CI orphan check | **Not implemented.** |
+| `check-domain-e2e-coverage.mjs` | Still not wired into any workflow (`check-coverage-matrix.mjs` runs in `pr-quality.yml`). |
+
+### §4/§A missing-test items: unchanged
+
+The restructure moved *where* suites run; it added no tests. Re-verified the P0s are still
+untested: `command_checks.rs`, `path_checks.rs`, `encryption/core.rs` have zero `#[test]`s and
+`delete_source_rpc` has none. Everything in §4 and Appendix A.3 stands.
+
+### NEW gaps introduced by the two-lane model
+
+1. **Playwright is decorative even at the release gate.** `playwright-e2e` has
+   `continue-on-error: true` (TODO #3615), and a job with `continue-on-error` reports `success`
+   to `needs`, so "Release CI Gate" can never fail on it. The whole web-E2E lane is currently a
+   can-never-fail check — the same anti-pattern as the gmail-flow specs, one level up. Either fix
+   the flaky specs and drop the flag, or exclude the job from the gate's `needs` and say so.
+2. **Cross-domain regressions land on `main` undetected.** The fast lane's own script comments
+   state it: coverage/tests scoped to the changed domain only — a change in domain A that breaks
+   domain B's tests, or breaks `tests/*.rs` integration behavior, merges green and only fails on
+   the release PR, where failures from many merged PRs arrive **batched** and need bisection.
+   Mitigations: treat a red release PR as build-cop priority with a revert-first policy; and/or a
+   scheduled full unit run on `main` (cheaper than per-PR, catches breakage within hours with
+   per-commit granularity via `git bisect` against a known-good tag).
+3. **Rust integration tests (`tests/*.rs`) never run on the PR lane for `src/**` changes** —
+   `rust-coverage-changed.sh` maps `src/<a>/<b>` to unit-test filters and only runs a `--test`
+   target when the *test file itself* changed. An RPC-behavior regression in a domain with thin
+   unit tests but good E2E coverage sails through the fast lane. Consider mapping changed domains
+   to their obvious integration targets too (e.g. `src/core/**` → `--test json_rpc_e2e`).
+4. **`vitest related` is import-graph-based**: tests coupled to a changed file through non-import
+   seams (mock fixtures, runtime registration, `?raw` prompt assets) won't be selected. The
+   0%-lcov backstop protects *changed lines lacking any test*, not *existing tests that would now
+   fail*. Low frequency, but worth knowing when a release-PR failure looks "impossible".
+
+### Phase 0 (updated)
+
+Unchanged items: wire orphaned tests, add `scripts/**` to PR-lane filters (now also: make
+`scripts/mock-api/**` changes at least trigger the scripts self-test job), WDIO reset hook,
+orphan-check, controller-domain check. **New:** resolve the Playwright `continue-on-error` gate
+bypass (#3615) and adopt a red-release-PR policy (build-cop + revert-first).
+
+---
+
+## 6. Strategy: how to get a lot of functional tests cheaply
+
+The audit shows the marginal value is in **behavioral/functional tests over shared harnesses**,
+not more per-file unit tests. Concrete leverage:
+
+1. **Table-driven contract suites.** The repo's biggest waste is copy-paste template families:
+   11 connector WDIO specs, 4 scanner-registry suites, ~4 allowlist tests × N channel providers,
+   6× registry-composition patterns in `tools/ops_tests.rs`, ~15 `all_schemas_returns_N` files.
+   Build shared helpers/macros once (`runConnectorContract()`, `allowlist_contract_tests!`,
+   `assert_schema_controller_parity()`, generic `ScannerRegistry<T>`) — each collapses dozens of
+   declarations while *increasing* consistency (some copies have drifted, e.g. one provider's
+   allowlist suite missing the case-sensitivity check others have).
+2. **The JSON-RPC E2E lane is the functional sweet spot.** Follow the repo's own ladder
+   (Rust → RPC → UI): most new functional coverage should be `tests/*.rs` against the mock —
+   cheap, deterministic, cross-domain. Priority suites: security-gate matrix over RPC, approval
+   park/decide/TTL, encryption round-trip, memory ingest→recall→delete_source, wallet/web3
+   quote→execute.
+3. **Property-based tests where inputs are adversarial** (proptest — currently absent):
+   `classify_command` ("never Read for a command containing an unescaped shell operator; never
+   panics"), path validation with symlink chains, encryption round-trip, tunnel framing
+   reassembly, webhook parsers. One property replaces dozens of hand-enumerated cases.
+4. **Fuzzing the untrusted-input parsers** (cargo-fuzz — currently absent): webhook JSON decode,
+   tunnel frame decode, deep-link URL parse. Subsumes most of the missing hostile-payload unit
+   tests at far higher coverage-per-effort; run time-boxed in CI.
+5. **Mutation testing to find vacuous tests systematically.** Auditors hand-found "asserts
+   nothing" tests one at a time; `cargo-mutants` scoped to the P0 zones (security/policy,
+   encryption, approval, webhooks) turns that into a measured, ranked list. Consider Stryker on
+   `app/src/services/` later.
+6. **One long journey spec per lane** (§4 P1) instead of many more element-visible smoke specs.
+7. **Fixture discipline over test branching:** the gmail-flow "if UI missing, pass" pattern is the
+   anti-pattern to ban — deterministic mock seeding, then unconditional assertions. Consider a
+   lint/review rule: no `console.log`-and-return early exits in specs.
+
+---
+
+## 7. Systemic improvements (from the completeness critic)
+
+Dimensions the suite (and this audit) currently have **zero** coverage of:
+
+| Dimension | State | Action |
+|---|---|---|
+| Test→CI orphan detection | Pester + scripts tests confirmed orphaned; no systematic map | Extend `scripts/generate-test-inventory.mjs` to assert every discovered test file is invoked by ≥1 CI job; fail on orphans. **Highest-leverage systemic fix.** |
+| Mutation testing | None (no cargo-mutants/Stryker) | Scoped cargo-mutants run on P0 zones. |
+| Property-based testing | None (no proptest/quickcheck) | §6.3 targets. |
+| Fuzzing | None (no cargo-fuzz targets) | §6.4 targets. |
+| Accessibility | Zero a11y assertions in the whole frontend | jest-axe smoke lane over ~10 key screens (approval card, recovery phrase, onboarding, composer). |
+| Performance/load | No benchmarks anywhere (no criterion/k6) | Start with criterion micro-benches on memory ingest + embeddings; defer load tests. |
+| Flake quantification | WDIO: maxInstances 1, no retries, order-dependent state; flake rate unmeasured | Pull CI rerun/failure history, rank flaky specs; land the per-spec mock reset (§5.4). |
+| Test-speed budget | Unknown critical path; 30s vitest timeout can hide sleeps | Emit per-suite duration report from the inventory generator; slowest-N list + soft budget. |
+| Upgrade/migration fixtures | migrate_openclaw/hermes logic exists; no prior-version snapshot test | Check in a prior-version workspace/DB/config snapshot; assert forward migration with no data loss. |
+| Local↔CI parity | Different E2E drivers per OS; GGML_NATIVE=OFF only locally | Document which specs run where; fold into the lane-parity report (§4 P2). |
+| Concurrency as a category | Races found anecdotally only | Start with the approval double-decide + event-bus tests; consider loom for the event bus later. |
+
+---
+
+## 8. Execution plan
+
+**Phase 0 — CI substrate (hours, do immediately)**
+- Wire orphaned tests into CI (`test:scripts`, socket auth/transport, Pester lane).
+- Add `scripts/**` to coverage-gate path filters.
+- Unconditional mock `/__admin/reset` per spec in WDIO.
+- Orphan-check in the inventory generator (test file → CI job mapping).
+- Controller-domain coverage check: every domain in `src/core/all.rs` referenced by ≥1 file in
+  `tests/` (Appendix A.4).
+
+**Phase 1 — P0 coverage (1–2 weeks)**
+- Security gate-matrix suite (command_checks/path_checks). 
+- Encryption core round-trip + tamper suite; RPC-integrated variant.
+- `delete_source_rpc` cascade suite.
+- Event-bus panic isolation; webhook flood behavior (or file the product gap).
+- `stop_core_process` debug command + crash-recovery E2E; RPC auth-failure E2E.
+
+**Phase 2 — deletions & rewrites (parallel with Phase 1, low risk)**
+- Land §2.1 deletions and §2.2 connector consolidation.
+- Rewrite §3 overfits (preserve the load-bearing assertions flagged by the skeptic pass).
+- Shared helpers: `assert_schema_controller_parity()`, `allowlist_contract_tests!`, envelope-unwrap
+  test helper, connector contract runner.
+
+**Phase 3 — P1 backlog (2–4 weeks, interleave with feature work)**
+- Approval×turn integration, TransportManager race, socket backoff, hostile webhook payloads,
+  path_scope invariant, web3 tool wiring, threadSlice/accountsSlice, journey spec, Playwright
+  approval mirror.
+
+**Phase 4 — new dimensions (ongoing)**
+- proptest + cargo-fuzz targets; scoped cargo-mutants; jest-axe lane; migration fixture;
+  duration report; mock chaos modes + contract tests.
+
+---
+
+## Appendix A — Rust integration/E2E slice
+
+*(Re-audited separately after the original workflow auditor returned an unusable result. These
+findings did not go through the skeptic-verify pass — treat as ⚠️ plausible; spot-check before
+deleting anything.)*
+
+**Summary.** The 129-file suite is dominated by `*_roundNN_raw_coverage_e2e.rs` files (coverage
+sprints, rounds 13–26) that re-exercise the same internal helpers (`app_state::snapshot`,
+`AuthProfilesStore`, `threads::ops`, `memory_sources::sync`) across 5–8 files under different
+names — **in-process calls, not RPC** — i.e. unit tests mislabeled `_e2e`. Real transport coverage
+(bearer auth, 401s, public-path bypass, SSE auth, `/schema`) in `json_rpc_e2e.rs` is genuinely good.
+
+### A.1 Consolidate / drop
+
+- **The same app_state quarantine invariant is asserted in ~5 files** (`near90_closure_…`,
+  `config_auth_app_state_connectivity_e2e.rs` ×2, `…round26…`, `config_credentials_raw_coverage…`).
+  Keep the `config_auth_app_state_connectivity_e2e.rs` pair; drop the rest.
+- **Four whole files are in-process unit tests wearing `_e2e` names**
+  (`app_credentials_threads_round24_…`, `…sources_round26_…`, `…memory_sources_raw_coverage…`,
+  `composio_credentials_state_raw_coverage_e2e.rs`): identical import quartet, no
+  `build_core_http_router`, no `/rpc` call. Fold their unique branch cases into the owning domains'
+  unit test modules; the RPC-level equivalents already exist in `json_rpc_e2e.rs`.
+- **`x402_twit_sh_live.rs` / `live_routing_e2e.rs`** (`#[ignore]`, need real wallet/backend):
+  contribute zero CI coverage but the x402 one is the *only* coverage artifact for the money-moving
+  `X402RequestTool` (skeptic-confirmed keep in §2.3). Reconciled action: **move to a `tests/manual/`
+  dir or Cargo feature gate** so they stop inflating the E2E count, and add mock-backed variants of
+  the same assertions so the code paths actually run in CI.
+
+### A.2 Overfitted
+
+- `config_auth_app_state_connectivity_e2e.rs::worker_a_controller_schemas_are_fully_exposed` —
+  exact-match hardcoded lists of 40+ RPC method names across 5 namespaces; any additive method
+  breaks it. Fix: must-exist superset assertion, or diff against the live registry; or `insta`
+  snapshots so additions are a `cargo insta review`, not a hand-edit.
+- `composio_raw_coverage_e2e.rs::…covers_all_declared_functions` — (also §3) claims "all" but never
+  checks `expected.len()` against the live catalog, so a new composio RPC silently gets zero
+  coverage while the test stays green. Assert catalog-length parity so omissions fail loudly.
+
+### A.3 Missing (adds to §4)
+
+- **P1 — ~20 registered controller domains with zero references anywhere in `tests/`**, including
+  real backend-facing surface: `recall_calendar`, `tinyplace`, `redirect_links`,
+  `desktop_companion`, `devices`, `announcements`, `provider_surfaces`, `people`,
+  `council_registry`, `audio_toolkit`, `agent_experience`, `http_host`, `skill_runtime`,
+  `session_import`. Minimum: one RPC round-trip (primary read + write) per domain via
+  `build_core_http_router`.
+- **P1 — SSE `/events` payload delivery**: auth is well tested; nothing asserts an actual
+  `DomainEvent` published via `publish_global` arrives as a correctly-shaped SSE frame.
+- **P1 — embedded-core crash mid-RPC**: `ollama_lifecycle_e2e.rs` covers the Ollama sidecar and
+  `port_conflict_recovery…` covers port collisions, but nothing kills the serving task mid-request.
+  (Pairs with the §4 P0 frontend crash-recovery E2E.)
+- **P2 — concurrent RPC cross-talk**: fire 20+ parallel mixed-method `/rpc` requests, assert each
+  response `id` matches its request.
+- **P2 — token rotation mid-session**: an open `/ws/dictation` or SSE stream outliving a bearer
+  rotation should terminate, not silently continue.
+
+### A.4 Ideas
+
+- Reorganize `raw_coverage` sprint files into per-domain modules — the "round" layout is exactly
+  why 20 uncovered domains went unnoticed.
+- **CI check: every controller domain registered in `src/core/all.rs` must have ≥1 reference in
+  `tests/`** — small script in the spirit of `generate-test-inventory.mjs`; would have caught all
+  of A.3 automatically. (Add to Phase 0 alongside the orphan-test check.)