diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 606f8293d5..c0839426e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,7 @@ jobs: mcp: ${{ steps.filter.outputs.mcp }} mcpCliHarness: ${{ steps.filter.outputs.mcpCliHarness }} engine: ${{ steps.filter.outputs.engine }} + discoveryIndex: ${{ steps.filter.outputs.discoveryIndex }} miner: ${{ steps.filter.outputs.miner }} minerTestHarness: ${{ steps.filter.outputs.minerTestHarness }} rees: ${{ steps.filter.outputs.rees }} @@ -107,6 +108,13 @@ jobs: - 'packages/loopover-miner/**' - 'scripts/check-miner-package.mjs' - 'package-lock.json' + # No dedicated build/pack script (unlike mcp/engine/miner above) -- discovery-index is a normal + # vitest-covered workspace package (packages/discovery-index/src/**/*.ts is in vitest.config.ts's + # coverage.include), so this filter only needs to feed validate-code's overall gate + validate-tests + # below, not a standalone job. + discoveryIndex: + - 'packages/discovery-index/**' + - 'package-lock.json' # 6 of the 7 MCP CLI-cluster test files, and ONLY these 6, are truly self-contained w.r.t. root # src/**: verified by direct-import inspection that test/unit/mcp-cli-*.test.ts (5 files) and # their shared test/unit/support/mcp-cli-harness.ts import nothing but node:* builtins + vitest, @@ -174,7 +182,7 @@ jobs: needs: changes # draft != true also gates push runs correctly: github.event.pull_request is unset there, so the # property access evaluates to null, and null != true is true. - if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true')) }} + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.discoveryIndex == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true')) }} runs-on: ubuntu-latest timeout-minutes: 45 env: @@ -544,13 +552,18 @@ jobs: validate-tests: name: validate-tests needs: changes - # Also runs on REES-only changes: Codecov holds codecov/patch until `after_n_builds` (6) coverage uploads - # land (codecov.yml), and REES-only PRs don't otherwise touch `backend`, so without the shards a REES PR - # would upload only its single `rees` report and the patch status would never post -- the same vacuous - # check this issue fixes. Running the shards guarantees the 6 uploads so Codecov posts a real patch verdict - # that now includes the review-enrichment/** diff (covered by the `rees` flag from validate-code). - # Draft PRs still skip the whole fan-out (#6448), so the REES trigger only applies to ready PRs. - if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.rees == 'true')) }} + # Also runs on REES/mcp/engine/miner/discoveryIndex-only changes: Codecov holds codecov/patch until + # `after_n_builds` (6) coverage uploads land (codecov.yml), and a PR scoped to just one of those + # self-contained packages doesn't otherwise touch `backend`, so without the shards it would upload no + # coverage report at all and codecov/patch would never post -- not even a vacuous pass, no check at + # all, and the required `validate` aggregate treats a skipped job as success (#7318-#7321: 3 + # packages/loopover-miner/lib-only PRs merged this way with validate-tests never running). Running the + # shards guarantees the 6 uploads so Codecov posts a real patch verdict covering whichever package + # actually changed (review-enrichment/** via the `rees` flag from validate-code; packages/loopover-mcp, + # packages/loopover-engine, packages/loopover-miner, and packages/discovery-index via their entries in + # vitest.config.ts's coverage.include, exercised by the root test/** suite these shards already run). + # Draft PRs still skip the whole fan-out (#6448), so these triggers only apply to ready PRs. + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.discoveryIndex == 'true')) }} runs-on: ubuntu-latest timeout-minutes: 20 strategy: @@ -731,8 +744,9 @@ jobs: validate-tests-merge: name: validate-tests-merge needs: [changes, validate-tests] - # Same REES trigger as validate-tests: when shards ran for a REES-only PR, merge their reports too. - if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.rees == 'true')) }} + # Same trigger as validate-tests: whenever shards ran (REES/mcp/engine/miner/discoveryIndex-only PRs + # included), merge their reports too. + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.discoveryIndex == 'true')) }} runs-on: ubuntu-latest timeout-minutes: 10 steps: diff --git a/codecov.yml b/codecov.yml index bdbeb7ef5a..0049f138eb 100644 --- a/codecov.yml +++ b/codecov.yml @@ -57,9 +57,10 @@ comment: require_changes: false # Coverage is collected by vitest (v8) over src/**, packages/loopover-engine/src/**, -# packages/loopover-miner/lib/**, and packages/discovery-index/src/** (plus review-enrichment/src/** via -# the standalone `rees` flag, c8 over its built dist remapped through source maps); mirror vitest's -# exclusions here so the Codecov total matches the local report. +# packages/loopover-miner/{lib,bin}/**, packages/loopover-mcp/{lib,bin}/**, and +# packages/discovery-index/src/** (plus review-enrichment/src/** via the standalone `rees` flag, c8 over +# its built dist remapped through source maps); mirror vitest's exclusions here so the Codecov total +# matches the local report. ignore: - "src/env.d.ts" - "apps/**" @@ -77,3 +78,12 @@ ignore: # discovery-index's process entrypoint (#7164) -- same "Docker build+boot, not unit-coverable" reasoning # as src/server.ts above; app.ts (everything it wires together) is what tests actually import. - "packages/discovery-index/src/server.ts" + # NOTE: packages/loopover-miner/bin/loopover-miner.ts and (once #7291 migrates it) + # packages/loopover-mcp/bin/loopover-mcp.ts are plain CLI dispatchers (no exports, terminate via + # process.exit()) exercised only via real subprocess spawn (test/unit/support/{miner,mcp}-cli-harness.ts), + # which v8 coverage can't instrument -- the same shape as src/server.ts above. They are deliberately NOT + # listed here: test/unit/codecov-policy.test.ts (#4864) requires packages/loopover-miner to never get a + # blanket ignore-list exemption, precisely so a change to the dispatcher forces either real coverage or a + # testable-export refactor (the path bin/loopover-miner-mcp.ts's createMinerMcpServer already took) + # instead of a silent pass. Both files are still in vitest.config.ts's coverage.include (so a PR that + # touches one now genuinely needs to satisfy codecov/patch on it, not skip grading entirely). diff --git a/vitest.config.ts b/vitest.config.ts index 68275da0bc..d638265aca 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -34,7 +34,32 @@ export default defineConfig({ // v8's coverage provider remaps through the inline sourcemap tsc emits, attributing coverage to // the .ts source instead -- this entry is what keeps that remapped file in the report. "packages/loopover-miner/lib/**/*.ts", + // bin/loopover-miner-mcp.ts exports createMinerMcpServer, imported in-process by + // test/unit/miner-mcp-*.test.ts -- genuinely unit-coverable, same as lib/ above. Its sibling + // bin/loopover-miner.ts (the plain CLI dispatcher: no exports, subprocess-only tested via + // test/unit/support/miner-cli-harness.ts) is NOT ignore-listed in codecov.yml -- test/unit/ + // codecov-policy.test.ts (#4864) forbids a blanket exemption for packages/loopover-miner, so it + // stays included and genuinely graded (near-0% today) until it either gains real in-process tests + // or is refactored into a testable export the way bin/loopover-miner-mcp.ts already was. + "packages/loopover-miner/bin/**/*.js", + "packages/loopover-miner/bin/**/*.ts", "packages/discovery-index/src/**/*.ts", + // packages/loopover-mcp/lib/*.js are plain JS today; issue #7291 migrates them to real TypeScript, + // keeping the same .js/.ts/.d.ts triplet shape packages/loopover-miner/lib/** already has (so + // coverage is wired BEFORE that PR lands, not as a follow-up fix). 4 of the 5 files + // (format-table/local-branch/redact-local-path/telemetry) are already imported in-process by + // test/unit/*.test.ts; lib/cli-error.js currently has no in-process test at all, so it will need + // one before a PR touching it can pass codecov/patch -- that's intended enforcement, not a bug. + "packages/loopover-mcp/lib/**/*.js", + "packages/loopover-mcp/lib/**/*.ts", + // packages/loopover-mcp/bin/loopover-mcp.js (~6,600 of ~7,400 lines in the package) is tested + // exclusively via subprocess spawn (test/unit/mcp-cli-*.test.ts, mcp-discovery.test.ts et al, + // through test/unit/support/mcp-cli-harness.ts's execFileSync/StdioClientTransport). Same shape as + // packages/loopover-miner/bin/loopover-miner.ts above -- and, consistent with that file NOT getting + // a codecov.yml exemption (#4864), this isn't ignore-listed either: #7291 (the TS migration) will + // need real in-process tests or a testable-export refactor for this file, not a free pass. + "packages/loopover-mcp/bin/**/*.js", + "packages/loopover-mcp/bin/**/*.ts", // review-enrichment is a standalone (non-workspace) package with its own node:test suite; its // coverage is collected separately via `npm run rees:coverage` (c8 over the built dist, remapped // to src through source maps) and uploaded to Codecov under the `rees` flag -- vitest never runs @@ -47,8 +72,18 @@ export default defineConfig({ // // packages/loopover-miner/lib/**/*.ts (above) also glob-matches its own still-hand-maintained // *.d.ts siblings (a ".d.ts" path ends in ".ts" too) -- those aren't real modules and can't be - // parsed as coverage source, so they're excluded the same way src/env.d.ts already is. - exclude: ["src/env.d.ts", "apps/**", "packages/discovery-index/src/server.ts", "packages/loopover-miner/lib/**/*.d.ts"], + // parsed as coverage source, so they're excluded the same way src/env.d.ts already is. Same story + // for the *.ts entries under packages/loopover-miner/bin/** and packages/loopover-mcp/{lib,bin}/** + // added above -- each glob-matches its own *.d.ts siblings too. + exclude: [ + "src/env.d.ts", + "apps/**", + "packages/discovery-index/src/server.ts", + "packages/loopover-miner/lib/**/*.d.ts", + "packages/loopover-miner/bin/**/*.d.ts", + "packages/loopover-mcp/lib/**/*.d.ts", + "packages/loopover-mcp/bin/**/*.d.ts", + ], // Emit lcov for Codecov to compute patch (changed-lines) coverage. reporter: ["text", "lcov"], // The 99% requirement now lives in codecov.yml as a *patch* gate (changed