diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index 002e5e743..6fc6518fb 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -58,6 +58,13 @@ jobs: # scripts/ci/rust-coverage-changed.sh (domain-scoped llvm-cov). rust-core-src-files: ${{ steps.filter.outputs['rust-core-src_files'] }} rust-tauri: ${{ steps.filter.outputs['rust-tauri'] }} + # Mock backend + shared scripts → run the scripts self-test suite. + scripts: ${{ steps.filter.outputs.scripts }} + # Test wiring surface (workflows, package.json, all.rs, tests/, script + # tests) → run the test-inventory orphan + controller-domain guard. + inventory: ${{ steps.filter.outputs.inventory }} + # Windows installer or its unit test → run the Pester install lane. + install-ps1: ${{ steps.filter.outputs.install_ps1 }} coverage: ${{ steps.filter.outputs.frontend == 'true' || steps.filter.outputs['rust-core'] == 'true' || steps.filter.outputs['rust-tauri'] == 'true' }} steps: - name: Checkout code @@ -144,6 +151,30 @@ jobs: - 'rust-toolchain.toml' - 'app/src-tauri/**' - 'scripts/ci-cancel-aware.sh' + # Shared mock backend + script self-tests. The mock backend every + # E2E suite depends on had zero PR-lane coverage before this (its + # own socket-auth tests never ran in CI) — arm the scripts + # self-test job on any mock/script change. + scripts: + - '.github/workflows/pr-ci.yml' + - 'scripts/mock-api/**' + - 'scripts/*.mjs' + - 'scripts/__tests__/**' + # Test-wiring surface: a change here can orphan a test or add an + # untested controller domain → run the inventory guard. + inventory: + - '.github/workflows/**' + - 'package.json' + - 'src/core/all.rs' + - 'tests/**' + - 'scripts/**/*.test.mjs' + - 'scripts/tests/**' + - 'scripts/generate-test-inventory.mjs' + # Windows installer + its Pester unit test. + install_ps1: + - '.github/workflows/pr-ci.yml' + - 'scripts/install.ps1' + - 'scripts/tests/OpenHumanWindowsInstall.Tests.ps1' frontend-checks: name: Frontend Checks (quality, i18n, docs, coverage) @@ -462,6 +493,87 @@ jobs: retention-days: 7 if-no-files-found: error + scripts-tests: + name: Scripts Self-Tests (mock backend + CLIs) + needs: [changes] + if: needs.changes.outputs.scripts == 'true' + runs-on: ubuntu-22.04 + timeout-minutes: 15 + container: + image: ghcr.io/tinyhumansai/openhuman_ci:rust-1.93.0 + steps: + - name: Checkout code + uses: actions/checkout@v5 + with: + fetch-depth: 1 + persist-credentials: false + + - name: Cache pnpm store + id: pnpm-cache + uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 + with: + path: .pnpm-store + key: pnpm-store-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: | + pnpm-store-${{ runner.os }}- + + # The socket self-tests resolve socket.io-client from app/, so the + # workspace deps must be installed. + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Save pnpm store cache + if: always() && steps.pnpm-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 + with: + path: .pnpm-store + key: ${{ steps.pnpm-cache.outputs.cache-primary-key }} + + - name: Run mock-backend + script self-tests + run: pnpm test:scripts + + test-inventory: + name: Test Inventory (orphan + controller-domain guard) + needs: [changes] + if: needs.changes.outputs.inventory == 'true' + runs-on: ubuntu-22.04 + timeout-minutes: 10 + container: + image: ghcr.io/tinyhumansai/openhuman_ci:rust-1.93.0 + steps: + - name: Checkout code + uses: actions/checkout@v5 + with: + fetch-depth: 1 + persist-credentials: false + + # Node-only guard (no workspace deps): asserts every script-level test is + # invoked by a package.json script/workflow, and every controller domain + # in src/core/all.rs is referenced under tests/. + - name: Run test-inventory guard + run: pnpm test:inventory + + pester-install: + name: PowerShell Install Test (Pester) + needs: [changes] + if: needs.changes.outputs['install-ps1'] == 'true' + # GitHub-hosted ubuntu-latest ships pwsh; the test only dot-sources + # install.ps1 helpers (no MSI, cross-platform), so no Windows runner needed. + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout code + uses: actions/checkout@v5 + with: + fetch-depth: 1 + persist-credentials: false + + # Invoked directly (not via `pnpm test:install-ps1`) so the lane needs no + # Node/pnpm toolchain — pwsh is preinstalled on ubuntu-latest. Keep this + # command in sync with the `test:install-ps1` package.json script. + - name: Run install.ps1 unit tests + run: pwsh -NoProfile -File scripts/tests/OpenHumanWindowsInstall.Tests.ps1 + pr-ci-gate: name: PR CI Gate needs: @@ -470,6 +582,9 @@ jobs: - rust-quality - rust-core-coverage - rust-tauri-coverage + - scripts-tests + - test-inventory + - pester-install if: always() runs-on: ubuntu-latest timeout-minutes: 15 @@ -483,6 +598,9 @@ jobs: ["Rust Quality"]="${{ needs['rust-quality'].result }}" ["Rust Core Coverage"]="${{ needs['rust-core-coverage'].result }}" ["Rust Tauri Coverage"]="${{ needs['rust-tauri-coverage'].result }}" + ["Scripts Self-Tests"]="${{ needs['scripts-tests'].result }}" + ["Test Inventory"]="${{ needs['test-inventory'].result }}" + ["PowerShell Install Test"]="${{ needs['pester-install'].result }}" ) failed=0 diff --git a/.github/workflows/release-ci.yml b/.github/workflows/release-ci.yml index 47ad21ec1..2e84a2113 100644 --- a/.github/workflows/release-ci.yml +++ b/.github/workflows/release-ci.yml @@ -309,7 +309,14 @@ jobs: - unit-tests - rust-e2e - build-playwright-e2e-artifact - - playwright-e2e + # `playwright-e2e` is intentionally EXCLUDED from the gate. It carries + # `continue-on-error: true` (flaky specs, TODO #3615), which makes the + # job report `success` to `needs` even when its steps fail — so listing + # it here would be a can-never-fail check that only reads as coverage. + # Rather than leave it decorative, we drop it from the gate until the + # specs are stabilised and the flag removed (#3615); the lane still runs + # for signal, and `build-playwright-e2e-artifact` (which CAN fail) stays + # gated so a broken web build is still caught. - e2e-desktop if: always() runs-on: ubuntu-latest @@ -318,11 +325,13 @@ jobs: - name: Require all Release CI jobs to pass run: | set -euo pipefail + # NOTE: "Playwright E2E" is deliberately absent — see the needs: + # comment above (#3615). Re-add it here once continue-on-error is + # removed from the playwright-e2e job. declare -A results=( ["Full Unit Suites"]="${{ needs['unit-tests'].result }}" ["Rust E2E"]="${{ needs['rust-e2e'].result }}" ["Build Playwright E2E Artifact"]="${{ needs['build-playwright-e2e-artifact'].result }}" - ["Playwright E2E"]="${{ needs['playwright-e2e'].result }}" ["Desktop E2E"]="${{ needs['e2e-desktop'].result }}" ) diff --git a/app/test/wdio.conf.ts b/app/test/wdio.conf.ts index d1d2dc477..a6f0dfbe6 100644 --- a/app/test/wdio.conf.ts +++ b/app/test/wdio.conf.ts @@ -34,6 +34,40 @@ const APPIUM_PORT = parseInt(process.env.APPIUM_PORT || '4723', 10); const CEF_CDP_HOST = process.env.CEF_CDP_HOST || '127.0.0.1'; const CEF_CDP_PORT = parseInt(process.env.CEF_CDP_PORT || '19222', 10); +// Admin base for the shared mock backend. The runner exports BACKEND_URL to +// the mock; fall back to the E2E_MOCK_PORT default the runner scripts use. +const MOCK_ADMIN_BASE = + process.env.BACKEND_URL || `http://127.0.0.1:${process.env.E2E_MOCK_PORT || 18473}`; + +// The mock backend carries module-level mutable state (conversations, cron +// jobs, webhook triggers, request log, socket sessions). Specs run in ONE +// ordered session and historically only reset it when a spec *remembered* to +// call `/__admin/reset` in its own hook — so a spec that failed before its +// reset poisoned the next spec file. Reset once at the start of every spec +// file, unconditionally, so no spec can leak mock state into the next one. +// Guarded by file path so nested `describe` blocks inside a file don't wipe +// state mid-file (specs still build up state across their own `it`s). +let lastResetSpecFile: string | null = null; + +async function resetMockBackendOncePerSpecFile(specFile: string | undefined): Promise { + if (!specFile || specFile === lastResetSpecFile) return; + lastResetSpecFile = specFile; + try { + const res = await fetch(`${MOCK_ADMIN_BASE}/__admin/reset`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: '{}', + }); + if (!res.ok) { + console.warn(`[wdio] mock /__admin/reset returned ${res.status} for ${specFile}`); + } + } catch (err) { + // Best-effort: some lanes run specs that don't stand up the mock backend. + // Never fail the run on an unreachable mock — just record it. + console.warn(`[wdio] mock /__admin/reset skipped for ${specFile}: ${(err as Error).message}`); + } +} + // appium-chromium-driver advertises support for platformName ∈ {windows, mac, linux} // (not "chromium" — that's only the automationName). Pick the actual OS so the // capability negotiation succeeds. @@ -134,6 +168,11 @@ export const config: Options.Testrunner & Record = { await browser.switchToWindow(target); } }, + beforeSuite: async function (suite: { file?: string }) { + // Fires once per Mocha suite. The per-file guard makes the reset run only + // for the first suite encountered in each spec file. + await resetMockBackendOncePerSpecFile(suite?.file); + }, afterTest: async function ( test: { title: string; parent?: string }, _context: unknown, diff --git a/package.json b/package.json index 6fd4636dc..9a7a0fd98 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,8 @@ "docs:generate": "node scripts/generate-architecture-docs.mjs", "docs:check": "node scripts/generate-architecture-docs.mjs --check", "docs:test": "node --test scripts/__tests__/generate-architecture-docs.test.mjs", + "test:scripts": "node --test scripts/__tests__/*.test.mjs scripts/mock-api/*.test.mjs scripts/mock-api/routes/__tests__/*.test.mjs", + "test:inventory": "node scripts/generate-test-inventory.mjs", "compile": "pnpm --filter openhuman-app compile", "dev": "pnpm --filter openhuman-app dev", "dev:app": "pnpm --filter openhuman-app dev:app", diff --git a/plan.md b/plan.md index 61ffb6a93..5fc0aa6de 100644 --- a/plan.md +++ b/plan.md @@ -280,12 +280,14 @@ untested: `command_checks.rs`, `path_checks.rs`, `encryption/core.rs` have zero 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) +### Phase 0 (updated) — ✅ landed -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). +Done in `ci/phase0-test-substrate`: wired orphaned tests (`test:scripts` + `scripts-tests` job, +`pester-install` job), added `scripts/mock-api/**` + `scripts/*.mjs` to PR-lane filters (mock +changes now trigger the scripts self-test job), WDIO per-spec-file `/__admin/reset` hook, +orphan-check + controller-domain check (`generate-test-inventory.mjs` → `test-inventory` job), +and resolved the Playwright `continue-on-error` gate bypass (#3615, excluded from the gate). +**Still open (process, not code):** adopt a red-release-PR policy (build-cop + revert-first). --- @@ -346,13 +348,20 @@ Dimensions the suite (and this audit) currently have **zero** coverage of: ## 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 0 — CI substrate (hours, do immediately)** — ✅ landed (PR: `ci/phase0-test-substrate`) +- [x] Wire orphaned tests into CI (`test:scripts` runs `scripts/__tests__`, `scripts/mock-api` + socket auth/transport + route tests; new `scripts-tests` PR-lane job) and a Pester lane + (`pester-install` job → `test:install-ps1`). +- [x] Add `scripts/mock-api/**` + `scripts/*.mjs` (+ `scripts/__tests__/**`) to the PR-lane path + filters so mock-backend/script changes trigger the scripts self-tests. +- [x] Unconditional mock `/__admin/reset` per spec file in WDIO (`beforeSuite`, file-path-guarded). +- [x] Orphan-check in the inventory generator (`scripts/generate-test-inventory.mjs`, wired as the + `test-inventory` PR-lane job via `test:inventory`): every script-level test file is invoked by + ≥1 package.json script or workflow. +- [x] Controller-domain coverage check: every domain in `src/core/all.rs` referenced by ≥1 file in + `tests/` (Appendix A.4). 26 currently-uncovered domains seeded into a burn-down allowlist. +- [x] Resolve the Release CI Gate Playwright `continue-on-error` bypass (§5bis item 1, #3615): + `playwright-e2e` excluded from the gate's `needs`/results with an explanatory comment. **Phase 1 — P0 coverage (1–2 weeks)** - Security gate-matrix suite (command_checks/path_checks). diff --git a/scripts/generate-test-inventory.mjs b/scripts/generate-test-inventory.mjs new file mode 100644 index 000000000..83880dfc1 --- /dev/null +++ b/scripts/generate-test-inventory.mjs @@ -0,0 +1,271 @@ +#!/usr/bin/env node +// Test-inventory guard: keeps the test suite honestly wired into CI. +// +// A test that no job runs is worse than no test — it reads as coverage while +// verifying nothing. This script enforces two ratchets that fail CI: +// +// (a) ORPHAN CHECK — every discovered script-level test file +// (`scripts/**/*.test.mjs` and the PowerShell install test) is invoked +// by >=1 package.json script (directly or via a `node --test `) +// OR referenced by a workflow. Framework-globbed suites (Vitest, WDIO, +// Playwright, cargo test) are discovered by their runners' own config +// globs, not enumerated here, so they are out of scope for the orphan +// check — the orphans the audit found all live under `scripts/`. +// +// (b) CONTROLLER-DOMAIN CHECK — every controller domain registered in +// `src/core/all.rs` (via `crate::openhuman::::all_*_controllers`) +// is referenced by >=1 file under `tests/`. Catches RPC domains that +// ship with zero integration/E2E coverage (recall_calendar, tinyplace, +// devices, …). +// +// Known-current offenders are seeded into the allowlists below so the check +// lands green; the intent is to burn those lists down over time. Any NEW +// offender (a fresh orphan test, or a new controller domain with no tests/ +// reference) fails CI until it is wired up or explicitly allowlisted. +// +// Usage: +// node scripts/generate-test-inventory.mjs # report + enforce +// node scripts/generate-test-inventory.mjs --check # same (explicit) +// node scripts/generate-test-inventory.mjs --json # machine-readable dump + +import fs from 'node:fs'; +import path from 'node:path'; + +const ROOT = process.cwd(); +const argv = new Set(process.argv.slice(2)); +const JSON_OUT = argv.has('--json'); + +// ───────────────────────────────────────────────────────────────────────────── +// Allowlists — burn these down. Adding an entry is a deliberate, reviewable act. +// ───────────────────────────────────────────────────────────────────────────── + +// Script-level test files permitted to lack any package.json/workflow invocation. +// Should stay empty: wire the test into `test:scripts` (or a dedicated script) +// instead of allowlisting it. +const ORPHAN_ALLOWLIST = new Set([]); + +// Controller domains permitted to lack any reference under tests/. Each entry +// is a Rust integration-coverage gap tracked in plan.md §4/§A.3 — remove the +// entry when the domain gains a tests/ reference. +const DOMAIN_ALLOWLIST = new Set([ + // Seeded 2026-07 from the initial run — plan.md §4/§A.3 Rust-E2E gaps. + // Burn down by adding an RPC round-trip under tests/ for each, then delete + // the corresponding line here. + 'agent_experience', + 'agent_meetings', + 'announcements', + 'audio_toolkit', + 'council_registry', + 'desktop_companion', + 'devices', + 'harness_init', + 'http_host', + 'mcp_audit', + 'memory_diff', + 'memory_goals', + 'model_council', + 'people', + 'plan_review', + 'provider_surfaces', + 'recall_calendar', + 'redirect_links', + 'referral', + 'session_import', + 'skill_runtime', + 'task_sources', + 'text_input', + 'thread_goals', + 'tinyplace', + 'webview_notifications', +]); + +// ───────────────────────────────────────────────────────────────────────────── +// Helpers +// ───────────────────────────────────────────────────────────────────────────── + +function walk(dir, predicate, out = []) { + if (!fs.existsSync(dir)) return out; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.name === 'node_modules' || entry.name === '.git') continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) walk(full, predicate, out); + else if (predicate(full)) out.push(full); + } + return out; +} + +function read(file) { + return fs.readFileSync(file, 'utf8'); +} + +function rel(file) { + return path.relative(ROOT, file).split(path.sep).join('/'); +} + +// Convert a shell-style glob (single-segment `*`, recursive `**`) into a RegExp +// anchored against a repo-relative POSIX path. +function globToRegExp(glob) { + let re = '^'; + for (let i = 0; i < glob.length; i++) { + const c = glob[i]; + if (c === '*') { + if (glob[i + 1] === '*') { + re += '.*'; + i++; + if (glob[i + 1] === '/') i++; // consume the trailing slash of `**/` + } else { + re += '[^/]*'; + } + } else if ('.+?^${}()|[]\\'.includes(c)) { + re += '\\' + c; + } else { + re += c; + } + } + return new RegExp(re + '$'); +} + +// ───────────────────────────────────────────────────────────────────────────── +// (a) Orphan check +// ───────────────────────────────────────────────────────────────────────────── + +function discoverScriptTests() { + const scriptsDir = path.join(ROOT, 'scripts'); + return walk(scriptsDir, (f) => f.endsWith('.test.mjs') || f.endsWith('.Tests.ps1')) + .map(rel) + .sort(); +} + +function loadInvocationSources() { + // Every package.json script command string. + const pkg = JSON.parse(read(path.join(ROOT, 'package.json'))); + const scriptCommands = Object.values(pkg.scripts ?? {}); + + // Every workflow YAML, raw. + const workflowsDir = path.join(ROOT, '.github', 'workflows'); + const workflowText = walk(workflowsDir, (f) => f.endsWith('.yml') || f.endsWith('.yaml')) + .map(read) + .join('\n'); + + return { scriptCommands, combinedText: scriptCommands.join('\n') + '\n' + workflowText }; +} + +// Extract glob/path args from each `node --test ...` occurrence in a command. +function nodeTestGlobs(command) { + const globs = []; + for (const m of command.matchAll(/node\s+--test\s+([^\n&|;]+)/g)) { + for (const tok of m[1].trim().split(/\s+/)) { + if (tok.startsWith('-')) continue; // skip flags like --test-reporter + globs.push(tok); + } + } + return globs; +} + +function computeOrphans(scriptTests) { + const { scriptCommands, combinedText } = loadInvocationSources(); + + // Files matched by a `node --test ` in any package.json script. + const globMatchers = scriptCommands.flatMap(nodeTestGlobs).map(globToRegExp); + + const orphans = []; + for (const file of scriptTests) { + const matchedByGlob = globMatchers.some((re) => re.test(file)); + const referencedLiterally = combinedText.includes(file); + const covered = matchedByGlob || referencedLiterally; + if (!covered && !ORPHAN_ALLOWLIST.has(file)) orphans.push(file); + } + return orphans; +} + +// ───────────────────────────────────────────────────────────────────────────── +// (b) Controller-domain check +// ───────────────────────────────────────────────────────────────────────────── + +function discoverControllerDomains() { + const allRs = read(path.join(ROOT, 'src', 'core', 'all.rs')); + const domains = new Set(); + // crate::openhuman::[::...]::all__(registered|internal)_controllers + const re = + /crate::openhuman::([a-z0-9_]+)(?:::[a-z0-9_]+)*::all_[a-z0-9_]+_(?:registered|internal)_controllers/g; + for (const m of allRs.matchAll(re)) domains.add(m[1]); + return [...domains].sort(); +} + +function domainsReferencedInTests() { + const testsDir = path.join(ROOT, 'tests'); + const combined = walk(testsDir, (f) => f.endsWith('.rs')) + .map(read) + .join('\n'); + return combined; +} + +function computeUnreferencedDomains(domains) { + const testsText = domainsReferencedInTests(); + const missing = []; + for (const domain of domains) { + const referenced = new RegExp(`\\b${domain}\\b`).test(testsText); + if (!referenced && !DOMAIN_ALLOWLIST.has(domain)) missing.push(domain); + } + return missing; +} + +// ───────────────────────────────────────────────────────────────────────────── +// Run +// ───────────────────────────────────────────────────────────────────────────── + +const scriptTests = discoverScriptTests(); +const orphans = computeOrphans(scriptTests); + +const domains = discoverControllerDomains(); +const unreferencedDomains = computeUnreferencedDomains(domains); +const referencedDomainCount = domains.length - unreferencedDomains.length - DOMAIN_ALLOWLIST.size; + +if (JSON_OUT) { + console.log( + JSON.stringify( + { + scriptTests, + orphans, + orphanAllowlist: [...ORPHAN_ALLOWLIST], + domains, + unreferencedDomains, + domainAllowlist: [...DOMAIN_ALLOWLIST], + }, + null, + 2, + ), + ); +} else { + console.log('Test inventory guard'); + console.log('===================='); + console.log(`Script-level test files discovered: ${scriptTests.length}`); + console.log(` orphaned (no invocation): ${orphans.length}`); + console.log(` allowlisted orphans: ${ORPHAN_ALLOWLIST.size}`); + console.log(`Controller domains in all.rs: ${domains.length}`); + console.log(` referenced in tests/: ${referencedDomainCount}`); + console.log(` allowlisted (known gaps): ${DOMAIN_ALLOWLIST.size}`); + console.log(` newly unreferenced: ${unreferencedDomains.length}`); +} + +let failed = false; + +if (orphans.length > 0) { + failed = true; + console.error('\n✖ Orphaned test files (invoked by no package.json script or workflow):'); + for (const file of orphans) console.error(` - ${file}`); + console.error(' Wire each into `test:scripts` (or a dedicated script), or allowlist with cause.'); +} + +if (unreferencedDomains.length > 0) { + failed = true; + console.error('\n✖ Controller domains registered in src/core/all.rs with no reference in tests/:'); + for (const domain of unreferencedDomains) console.error(` - ${domain}`); + console.error(' Add >=1 RPC round-trip under tests/, or allowlist in DOMAIN_ALLOWLIST with cause.'); +} + +if (failed) { + process.exit(1); +} + +if (!JSON_OUT) console.log('\n✔ All script tests are wired in and every controller domain is referenced in tests/.');