From 0bf1c2f2a279b57daf21307713c765d86bceb774 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 10 Jun 2026 18:20:17 -0400 Subject: [PATCH] ci: publish Go coverage badge + GitHub Code Quality PR comments (#133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve #133's coverage-reporting decision now the repo is public — self-hosted, no third-party SaaS: - README badge: a `cov badge` subcommand emits a shields.io endpoint JSON for the merged Go total (the exact number `make cov` gates); a new non-gating `badge` job publishes it to an orphan `badges` branch (sole holder of contents:write, main-push only) via scripts/ci/publish-badge.sh. README reads it over raw.githubusercontent.com. - PR drop comments: GitHub Code Quality (native, first-party). The coverage job converts the merged Go profile to Cobertura (go tool gocover-cobertura, excludes mirrored from .testcoverage.yml) and uploads via actions/upload-code-coverage; the github-code-quality[bot] posts the aggregate + per-file diff-vs-main comment. continue-on-error so this public-preview feature never reds CI; fork PRs skip. Gating is unchanged — `make cov`'s thresholds stay the only merge gate. actionlint doesn't know the preview `code-quality` scope yet, suppressed narrowly in .github/actionlint.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/actionlint.yaml | 13 ++++ .github/workflows/README.md | 36 ++++++++++- .github/workflows/ci.yml | 97 +++++++++++++++++++++++++++- .testcoverage.yml | 6 +- AGENTS.md | 4 +- CHANGELOG.md | 6 +- README.md | 3 + docs/src/content/docs/development.md | 2 +- go.mod | 2 + go.sum | 2 + scripts/ci/publish-badge.sh | 54 ++++++++++++++++ scripts/cov/main.go | 70 +++++++++++++++++++- 12 files changed, 282 insertions(+), 13 deletions(-) create mode 100644 .github/actionlint.yaml create mode 100755 scripts/ci/publish-badge.sh diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 00000000..848bb6f3 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,13 @@ +# actionlint configuration. +# +# The `coverage` job in ci.yml declares the `code-quality: write` token scope +# for GitHub Code Quality (the native PR coverage comment — #133). That scope +# is in public preview (added by GitHub 2026-05) and isn't yet in actionlint's +# built-in scope list, so actionlint v1.7.12 flags it as "unknown permission +# scope". Suppress only that exact message — a genuine scope typo (e.g. +# `contents` → `conten`) still produces a different message and fails. Drop +# this once actionlint ships the scope. +paths: + "**/*.yml": + ignore: + - 'unknown permission scope "code-quality"' diff --git a/.github/workflows/README.md b/.github/workflows/README.md index b901f0d5..e6867cd8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -24,6 +24,7 @@ graph TB unit -. "coverage-unit (poll)" .-> coverage integration -. "coverage-integration (poll)" .-> coverage e2e -. "coverage-e2e (poll)" .-> coverage + coverage --> badge["badge (main) — non-gating"] title["title (PRs)"] --> ci["CI (aggregator — sole required check)"] lint["lint"] --> ci coverage --> ci @@ -54,12 +55,14 @@ Break one of these knowingly or not at all. Go suites) and event-filtered jobs (title on pushes, deploys on PRs) never orphan the required check, and adding/renaming jobs never requires a ruleset edit. Consequence: every job that must gate merges - **must be in the aggregator's `needs` list**. Two jobs are deliberately - non-gating and excluded: `timing` (advisory wall-clock table) and + **must be in the aggregator's `needs` list**. Three jobs are deliberately + non-gating and excluded: `timing` (advisory wall-clock table), `docs-preview` (the convenience Cloudflare preview deploy — `docs-build` already validates the build and *is* a need, so only the build gates; the preview deploy reports its own "Docs preview" check but, slow or - failed, never delays or reds `CI`). + failed, never delays or reds `CI`), and `badge` (publishes the README + coverage badge to the `badges` branch on main pushes — a badge push must + never block a merge; it reports its own "Coverage badge" status). 2. **A dedicated `coverage` job applies the consolidated gate, polling — not `needs`-ing — the suites.** Each suite (`unit`, `integration`, @@ -124,6 +127,33 @@ Break one of these knowingly or not at all. cushion the next run), and concurrent same-key misses produce benign "already exists" warnings. +## Coverage publishing + +The `coverage` job both **gates** (`make cov` against the floors in +`.testcoverage.yml`) and **publishes** — independent concerns, and only the +gate blocks merges ([#133](https://github.com/Wave-RF/WaveHouse/issues/133)): + +- **Per-run job summary** — the merged per-package func table on every run's + Summary page. +- **PR comment (GitHub Code Quality)** — on same-repo PRs and main pushes the + job converts the merged Go profile to Cobertura (`go tool gocover-cobertura`, + `-ignore-dirs` mirroring `.testcoverage.yml`'s global excludes so the % tracks + the merged-total gate) and uploads it via `actions/upload-code-coverage`; the + `github-code-quality[bot]` posts the aggregate + per-file diff-vs-main + comment. **Non-gating**: the upload is `continue-on-error`, so this + public-preview feature can never red `CI`. Fork PRs skip (no `code-quality` + token, per GitHub's own guard). Renders only once the repo's *Settings → + Security → Code quality* is enabled. +- **README badge** — on main pushes the job emits a shields.io endpoint JSON + for the merged Go total (`cov badge`, the exact gated number); the separate + non-gating `badge` job publishes it to the orphan `badges` branch, which the + README badge reads over `raw.githubusercontent.com` (public-repo only). The + `badge` job is the sole holder of `contents:write` and runs only on trusted + main, so a push to `badges` can't be influenced by PR code (invariant 5). + +SDK (TS) coverage is gated by `make cov` but not yet published — extend with a +`language: javascript` upload step and a second badge JSON when wanted. + ## Merge queue PRs land through a **merge queue**: "Merge when ready" enqueues the PR, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0cc7e0f0..591820e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -321,6 +321,12 @@ jobs: permissions: contents: read actions: read # poll for + download the suites' coverage fragments from this run + # Upload the Cobertura report to GitHub Code Quality, which posts the + # PR coverage comment (aggregate + per-file diff vs main). Narrow + # scope: it can't read secrets or write contents. Public preview, so + # actionlint doesn't know the scope yet — suppressed in + # .github/actionlint.yaml. + code-quality: write steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: @@ -358,8 +364,56 @@ jobs: path: tmp/coverage - name: Render consolidated report + gate thresholds run: make cov - # Coverage in the job-summary panel. See #133 for post-launch - # reporting decisions. + # ── GitHub Code Quality: PR coverage comment (non-gating) ────────── + # Convert the merged Go profile to Cobertura and hand it to GitHub + # Code Quality, whose bot posts the PR coverage comment (aggregate + + # per-file gained/lost vs main). `!cancelled()` so a coverage *drop* + # that fails the gate above still gets a comment; the upload is + # continue-on-error, so this preview feature can never red CI — the + # gate stays `make cov`. The -ignore-dirs mirror .testcoverage.yml's + # global excludes so Code Quality's % tracks the merged-total gate. + - name: Convert Go coverage to Cobertura + if: ${{ !cancelled() }} + run: | + [ -f tmp/coverage/total/coverage.txt ] || exit 0 + go tool gocover-cobertura \ + -ignore-dirs '/(testutil|tests|scripts)(/|$)' \ + < tmp/coverage/total/coverage.txt > tmp/coverage/go-coverage.xml + # Skipped on fork PRs (no code-quality token), per GitHub's own guard: + # baseline on main pushes, comparison on same-repo PRs. + - name: Upload Go coverage to Code Quality + if: >- + ${{ !cancelled() && ( + (github.event_name == 'push' && github.ref == 'refs/heads/main') || + (github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository) + ) }} + continue-on-error: true + uses: actions/upload-code-coverage@abb5995db9e0199b0e2bb9dbd136fce4cb1ec4d3 # v1.3.0 + with: + file: tmp/coverage/go-coverage.xml + language: go + # ── Go coverage badge data (main only) ───────────────────────────── + # Emit the shields.io endpoint JSON for the merged Go total (the exact + # gated number) and hand it to the `badge` job, the sole holder of + # contents:write, which publishes it to the `badges` branch. + - name: Generate Go coverage badge + if: ${{ !cancelled() && github.event_name == 'push' && github.ref == 'refs/heads/main' }} + run: | + mkdir -p tmp/coverage/badge + go run ./scripts/cov badge > tmp/coverage/badge/coverage-go.json + - name: Upload badge data + if: ${{ !cancelled() && github.event_name == 'push' && github.ref == 'refs/heads/main' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: go-coverage-badge + path: tmp/coverage/badge + if-no-files-found: error + retention-days: 3 + overwrite: true + # Coverage in the job-summary panel — the at-a-glance per-run number + # (the README badge + Code Quality PR comment are the published + # surfaces; this stays for the full per-package func table). - name: Coverage summary if: always() run: | @@ -372,6 +426,42 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" fi + # ── Coverage badge publish (main only, non-gating) ───────────────── + # Publishes the Go merged-total coverage to the orphan `badges` branch as + # a shields.io endpoint JSON, which the README badge reads over + # raw.githubusercontent.com (#133). The ONLY job holding contents:write, + # and it runs solely on main pushes off a trusted-main checkout — it never + # executes PR code (trust-domain invariant). Non-gating: deliberately NOT + # in the `CI` aggregator's needs (a badge push must never block a merge) — + # it reports its own "Coverage badge" status. The number is produced by + # the coverage job (`cov badge`, the exact gated value) and passed here as + # the go-coverage-badge artifact, so this job needs no Go toolchain. + badge: + name: Coverage badge + needs: [changes, coverage] + if: >- + github.event_name == 'push' && github.ref == 'refs/heads/main' && + needs.changes.outputs.code == 'true' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: write # push the badge JSON to the `badges` branch + actions: read # download the go-coverage-badge artifact from this run + steps: + # persist-credentials stays at its default (true): this job's whole + # purpose is to push, and it runs only on trusted main. publish-badge.sh + # fetches the badges branch itself, so a shallow checkout is enough. + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + fetch-depth: 1 + - name: Download badge data + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: go-coverage-badge + path: tmp/coverage/badge + - name: Publish badge to the badges branch + run: scripts/ci/publish-badge.sh tmp/coverage/badge/coverage-go.json coverage-go.json + # ── Docs deploys ───────────────────────────────────────────────────── # Both deploy jobs check out the DEFAULT branch (or, on push, the pushed # main commit itself) and install wrangler from that trusted lockfile — @@ -565,7 +655,7 @@ jobs: # deploy (see its job), non-gating by design, so it neither delays nor # reds the required check — its own "Docs preview" status reports it. The # `docs-build` it depends on IS here, so a broken docs BUILD still gates. - # `timing` (non-gating) is the only other job excluded. + # `timing` and `badge` (both non-gating) are the only other jobs excluded. ci: name: CI needs: @@ -615,6 +705,7 @@ jobs: integration, e2e, coverage, + badge, docs-preview, docs-deploy, ] diff --git a/.testcoverage.yml b/.testcoverage.yml index 245136eb..1b81752a 100644 --- a/.testcoverage.yml +++ b/.testcoverage.yml @@ -2,8 +2,10 @@ # behind `make cov` / `make test-`) which renders profiles and # gates each suite against the numbers below. Schema is compatible with # `vladopajic/go-test-coverage`'s config format, but we don't run that -# action anymore — see issue #133 for the future-state coverage -# reporting decision (post-OSS-launch, likely Codecov or similar). +# action anymore. Publishing (#133) lives in CI's `coverage` job — a +# README badge from the merged Go total (`cov badge`) and PR comments +# via GitHub Code Quality; see .github/workflows/README.md "Coverage +# publishing". This file is the gate; publishing reads from it. # Merged-suites profile produced by `scripts/cov merge` at the end of # `make ci`. Contains unit + integration + e2e coverage stitched diff --git a/AGENTS.md b/AGENTS.md index ae86f52d..99d3ba5f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,7 +107,7 @@ Verbose: `V=1 make test`. Extra args: `make test ARGS="-run TestFoo"`. Build tag Tooling notes (the non-obvious bits `make help` won't tell you): -- Dev tools (`gotestsum`, `gofumpt`, `goimports`, `govulncheck`, `go-test-coverage`, `deadcode`, `gsa`, `goda`) are pinned in `go.mod` via `tool` directives — `go tool `, no manual install. +- Dev tools (`gotestsum`, `gofumpt`, `goimports`, `govulncheck`, `go-test-coverage`, `gocover-cobertura`, `deadcode`, `gsa`, `goda`) are pinned in `go.mod` via `tool` directives — `go tool `, no manual install. - `golangci-lint` is pinned in the Makefile (v2.11.4), auto-installed to `.bin/` on first `make lint` — kept out of `go.mod` (its deps conflict with the main module). - `pnpm` (≥ 11.1) + `Node 22 LTS` (`.nvmrc`, matches CI) must be on PATH; `make tools` runs one root `pnpm install --frozen-lockfile` across the three workspaces (SDK `clients/ts/`, E2E `tests/e2e/sdk/`, docs `docs/`). - **GNU Make 4+** required (uses `--output-sync=target`); macOS BSD Make 3.81 won't parse it. Full setup: `docs/src/content/docs/development.md` § Prerequisites. @@ -123,7 +123,7 @@ Tooling notes (the non-obvious bits `make help` won't tell you): - **Policy helpers**: Use `policy.NewMemoryStore(p)` for in-memory policy testing without NATS. - **Pipes helpers**: Use `pipes.NewMemoryStore(queries...)` for in-memory pipes testing without NATS. - **Response assertions**: Use `testutil.AssertJSONResponse(t, rec, status, expected)` and `testutil.AssertJSONContains(t, rec, status, substring)`. -- **Coverage target**: 80% project-wide (CI enforces `threshold.total` in `.testcoverage.yml` against the merged unit + integration + e2e profile). Per-suite minima also enforced: unit 80%, integration 20%, e2e 60%, sdk 50%. Aim for 80%+ on new code. +- **Coverage target**: 80% project-wide (CI enforces `threshold.total` in `.testcoverage.yml` against the merged unit + integration + e2e profile). Per-suite minima also enforced: unit 80%, integration 20%, e2e 60%, sdk 50%. Aim for 80%+ on new code. Coverage is published as a README badge (Go merged-total) and as PR comments via GitHub Code Quality — see `.github/workflows/README.md` "Coverage publishing"; the gate is unchanged. - **Every new function should have corresponding test cases.** Run `make lint` and `make test` before considering work complete. - **E2E tests via SDK**: The TypeScript SDK is the primary E2E test harness. Tests in `tests/e2e/sdk/` exercise the full pipeline (ingest → ClickHouse → query) and simultaneously validate backend behavior and SDK correctness. Use `make test-e2e` to run. Add new E2E scenarios as `tests/e2e/sdk/*.test.ts` files using helpers from `tests/e2e/sdk/helpers.ts`. - **Per-suite table isolation**: Each e2e test file owns its own ClickHouse tables — `clicks_` / `events_` / `users_`, generated from `tests/e2e/sdk/tables.ts` and created by `setup.ts`. A new test file must (1) add its suite name to `SUITES` in `tables.ts` and (2) get its names via `const T = suiteTables("")`, then reference `T.clicks` etc. — never a bare `clicks`. This makes cross-file *data* contamination structurally impossible. Files still run **sequentially** (`vitest.config.ts` `maxWorkers: 1`): running them in parallel is blocked by shared *global policy* state (several files read-modify-write the single policy document; `streaming.test.ts` flips the global `default_role`), so policy-mutating tests snapshot the full policy and restore it. Dropping `maxWorkers: 1` is a deferred follow-up tracked in #214 (per-table policy storage; see `docs/src/content/docs/ingest-pipeline.md` § Deferred). diff --git a/CHANGELOG.md b/CHANGELOG.md index e2818855..1ff0c046 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- **Coverage publishing — a self-hosted Go coverage README badge and GitHub Code Quality PR comments** (`.github/workflows/ci.yml`, `.github/actionlint.yaml` (new), `scripts/cov/main.go`, `scripts/ci/publish-badge.sh` (new), `.testcoverage.yml`, `go.mod`/`go.sum`, `README.md`, `AGENTS.md`, `docs/src/content/docs/development.md`): closes #133, now that the repo is public. Two published surfaces, both **non-gating** — `make cov`'s thresholds stay the only merge gate. (1) **README badge**: a new `cov badge` subcommand renders a [shields.io endpoint](https://shields.io/endpoint) JSON for the merged Go total using the *exact* number `threshold.total` gates (same `.testcoverage.yml` excludes), and a new non-gating `badge` job — the sole holder of `contents:write`, running only on trusted main — publishes it to an orphan `badges` branch via `scripts/ci/publish-badge.sh`, which the README reads over `raw.githubusercontent.com` (unrestricted for a public repo). (2) **PR comments**: the `coverage` job converts the merged Go profile to Cobertura (`go tool gocover-cobertura`, a new pinned Go `tool` dependency, with `-ignore-dirs` mirroring the YAML's global excludes) and uploads it to GitHub Code Quality via `actions/upload-code-coverage` (`code-quality: write`); the `github-code-quality[bot]` posts the aggregate + per-file diff-vs-`main` comment. The upload is `continue-on-error` so this public-preview GitHub feature can never red CI, and fork PRs skip it (no `code-quality` token, per GitHub's own guard). `actionlint` doesn't recognize the preview `code-quality` permission scope yet, so a new `.github/actionlint.yaml` suppresses only that one message. Requires the repo's *Settings → Code quality* enablement for the comments to render. Full design in `.github/workflows/README.md` §"Coverage publishing". + ### Changed - **CI is now a job DAG instead of one monolithic job, and the docs deploys no longer expose the Cloudflare token to PR-authored code** (`.github/workflows/ci.yml`, `.github/workflows/housekeeping.yml`, `.github/actions/setup-env/action.yml`, `Makefile`, `docs/wrangler.jsonc`, `AGENTS.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/claude-code.md`, `CONTRIBUTING.md`, `scripts/lint-pr-title.sh`, `.claude/hooks/agent-bash-gate.sh`): closes #305. The single `make ci` job becomes parallel jobs over the *same Makefile targets* (local `make ci` stays the dev mirror): `lint`, `unit`, `integration`, `e2e` (builds its own SDK dist + cover binary via `make -j test-e2e` on a warm per-suite cache and runs the suite exactly like a local run), `coverage` (a dedicated job that merges every suite's `coverage-` fragment and applies every threshold gate via `make cov` — like local `make ci`'s final step, so the gate is decoupled from the e2e suite; it's `needs: changes` only and *polls* for the fragments rather than `needs`-ing the suites, so its setup overlaps them and the merge fires ~10s after the last suite instead of serializing ~50s of setup onto the critical path), and `docs-build` (`make build-docs`, docs-affecting changes only, uploads the docs dist artifact the preview/deploy jobs consume) — public-repo runners are free and 4-core, so the pipeline spreads horizontally instead of queueing in one process. The architecture is documented once, in `.github/workflows/README.md` (DAG diagram, design invariants, cache key policy, add-a-job recipe, and the measured-but-deferred optimizations — e2e sharding among them), and the workflow's logic lives in shellcheck-gated scripts (`scripts/ci/` — `classify-changes.sh`, `check-pr-title.sh`, `docs-preview-comment.sh`, `timing-summary.sh`, `wait-artifact.sh`; over the shared, dependency-free path classifier `scripts/classify-paths.sh`, unit-tested by `scripts/classify-paths.test.sh` via `make test-classify-paths` and reused by the `pre-push` git hook so a docs/prose-only push requires only `make verify`, not a full `make ci` — the same suites CI skips for those changes) rather than inline YAML; caches are owned end-to-end by `setup-env` via nested `actions/cache` (automatic post-job saves — the per-job save-step boilerplate is gone); a non-gating `Timing summary` job writes a per-job wall-clock table to every run's Summary page; and `make verify` gains two leaves that gate the new surface area — `lint-sh` (shellcheck `v0.11.0`, checksum-verified install via `scripts/install-shellcheck.sh`) and `lint-gha` (actionlint `v1.7.12`) — so the CI plumbing is linted like any other source. The workflow also handles `merge_group` events (full suite against the merge-group ref), enabling a **merge queue** on `main`: the queue re-tests each PR against current main at landing time, which replaces the ruleset's "require branches to be up to date" rule — no more manual branch updates after every sibling merge. A new aggregator job named `CI` is the ruleset's **sole required status check** (it fails on any failed/cancelled job and counts skipped jobs as passing), so docs-only PRs skip the Go suites without orphaning the gate and future job changes never require ruleset edits. The PR-title (Conventional Commits) gate moves into the `PR title` job under that aggregator, validated by the same `scripts/lint-pr-title.sh` from a trusted `main` checkout; `PR housekeeping` (`pull_request_target`) drops to non-required and keeps what needs fork-PR write access — path labels, the sticky title-explainer comment, and a new nudge that re-runs the failed `PR title` job when a title edit fixes it (the job re-reads the title from the API, so no new push is needed). The **#305 fix**: docs previews/production deploys run in dedicated `docs-preview`/`docs-deploy` jobs that check out trusted `main` (wrangler, worker source, and config never resolve from the PR tree), consume only the static `docs/dist` artifact, and are the only jobs that reference `CLOUDFLARE_*` secrets; previews now publish right after `docs-build` instead of waiting on the full test pipeline, and the `docs-preview` deploy is **non-gating** — it's not in the `CI` aggregator's `needs` (only `docs-build` gates), so a slow or failed Cloudflare preview reports its own "Docs preview" check but never delays or reds the required check; production (`docs-deploy`, on the post-merge main push) still requires everything green. Per-job least-privilege permissions replace the old workflow-wide `contents: write`, and the Go build cache is partitioned per job (unit/integration/e2e compile with different flags) so each suite stays warm. @@ -181,7 +185,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Ingest worker no longer infinite-retries permanent delete errors** (`internal/ingest/bento.go`, `internal/ingest/bento_test.go`, `docs/src/content/docs/architecture.md`, `AGENTS.md`): `jsInput.Read`'s `action: "delete"` block called `m.Nak()` on every `chConn.Exec` failure, which JetStream interprets as "redeliver immediately." A delete whose error was *deterministic* (syntax error, unknown table, malformed identifier) would loop forever — clogging the buffer consumer, burning CPU, and spamming logs with the same message. Phase 1 of issue #91: every delete-Exec error is now treated as permanent. The original NATS envelope is published to `dlq.` (reusing the existing `bentoDLQDropped` counter when the DLQ publish itself fails) and the message is `DoubleAck`'d so it leaves the main queue. Issue #91 stays open after this lands as the Phase 2 tracker for transient-vs-permanent error classification (timeouts and network errors should still `Nak()` for retry); Phase 1 alone is the stopgap, Phase 2 makes the trade-off acceptable in production. *Superseded by the insert-only lock in `Removed` above — both the delete branch and the DLQ delete-envelope shape it required are gone.* - **CORS middleware: spec-compliant wildcard, no credentials, no header decoration on same-origin** (`internal/api/router.go`, `internal/api/router_test.go`, `AGENTS.md`, `config.yaml`, `docs/src/content/docs/configuration.md`, `docs/src/content/docs/deployment.md`): closes [#29](https://github.com/Wave-RF/WaveHouse/issues/29) and bookends [#30](https://github.com/Wave-RF/WaveHouse/issues/30). Three behavior changes, one rationale. (1) Dropped `Access-Control-Allow-Credentials: true` entirely — WaveHouse is a Bearer-token API (`Authorization: Bearer `), cookies are never used (verified: no `http.Cookie` / `SetCookie` anywhere in the Go tree, TS SDK sends no `credentials: 'include'`), so credentials mode is unnecessary AND the previous combination of `Allow-Credentials: true` with `Allow-Origin: *` violated the CORS spec — browsers reject that pairing, which silently broke any client that ever set `credentials: 'include'`. (2) Requests with no `Origin` header (same-origin browser navigation, server-to-server, curl) now skip the CORS decoration entirely instead of unconditionally stamping `Allow-Methods`/`Allow-Headers`/`Allow-Credentials` on every response. (3) A preflight from a disallowed origin still returns 204 but with no CORS headers, which the browser treats as preflight failure — same outcome as before but without leaking the methods/headers list to origins that aren't allowed. Allowlist mode sets `Vary: Origin` on both hits *and* rejects so shared caches can't memoize a headerless reject under the URL alone and replay it to a later allowed-origin request. Test coverage in `router_test.go` pins each branch: wildcard echoes `*`, allowlist hit echoes origin + `Vary: Origin`, allowlist miss gets no `Allow-Origin` but still gets `Vary: Origin` (both for regular and OPTIONS), no-Origin requests pass through clean, and a table-driven test asserts `Allow-Credentials` is never emitted across wildcard / empty-allowlist / allowlist-hit. Posture is documented as `AGENTS.md` §"Key Design Decisions" item 16 so future contributors don't reintroduce credentials or cookie auth without a design discussion. Config sample updated with the dev recipe (point at `http://localhost:3000` etc. instead of `*` once a frontend is built). - **OTel shutdown no longer hangs process exit when the collector is unreachable** (`cmd/wavehouse/main.go`): the `defer otelShutdown(context.Background())` was unbounded, and the OTel SDK's batch processors don't fully honor the shutdown context against an unreachable gRPC endpoint (the dial retries with backoff continue past the deadline). Bounded the shutdown to 5s. Discovered while writing `tests/integration/otel_test.go` `TestOTel_UnreachableEndpoint_DoesNotBlockStartupOrEmits`. -- **Coverage badge step no longer fails on every main push** (`.testcoverage.yml`, `AGENTS.md`): the `profile:` field pointed at `tmp/coverage/unit/coverage.txt` (~73% — `./internal/...` only) but `threshold.total` was 80, so `vladopajic/go-test-coverage`'s own gate failed unconditionally even after `make ci` had passed its per-suite + merged gates. Switched the action's profile to `tmp/coverage/total/coverage.txt` — the merged unit+integration+e2e profile `scripts/cov merge` produces at the end of `make ci` (~81% across the same files) — so the 80% gate applies to the project-wide number the badge advertises, not to a single suite. Per-suite gates in the YAML's `suites:` block continue to run inside `make ci` itself against the per-suite profiles before the badge step ever fires. AGENTS.md updated to state 80% project-wide minimum + per-suite minima. +- **Coverage badge step no longer fails on every main push** (`.testcoverage.yml`, `AGENTS.md`): the `profile:` field pointed at `tmp/coverage/unit/coverage.txt` (~73% — `./internal/...` only) but `threshold.total` was 80, so `vladopajic/go-test-coverage`'s own gate failed unconditionally even after `make ci` had passed its per-suite + merged gates. Switched the action's profile to `tmp/coverage/total/coverage.txt` — the merged unit+integration+e2e profile `scripts/cov merge` produces at the end of `make ci` (~81% across the same files) — so the 80% gate applies to the project-wide number the badge advertises, not to a single suite. Per-suite gates in the YAML's `suites:` block continue to run inside `make ci` itself against the per-suite profiles before the badge step ever fires. AGENTS.md updated to state 80% project-wide minimum + per-suite minima. *Superseded — #129 removed this `vladopajic/go-test-coverage` badge step entirely, and the `Added` entry at the top of this section (#133) replaces it with a self-hosted `cov badge` shields.io badge + GitHub Code Quality PR comments. The `profile: tmp/coverage/total/coverage.txt` setting introduced here still stands, now consumed by `scripts/cov`.* - **Dependabot auto-merge no longer slipped past CI on consolidated-pipeline PRs** (`main branch protection` ruleset; `.github/workflows/project-orchestrator.yml`; `.github/workflows/claude-review.yml`; `AGENTS.md`): on commit `93b3206` the previously six-job CI was collapsed into a single job named `CI`, but three places still referenced the old multi-job names — the ruleset's `required_status_checks` (`Validate` + `Admin approval` only — `CI` not listed), the `bot-clean` check in `project-orchestrator.yml` (looking for `Build`/`Validate`/`Lint`/`Test`/`Integration Tests`/`SDK Tests`), and `REQUIRED_CHECKS` in `claude-review.yml` (same six). Net effect: a Dependabot PR opened at T+0 would have its `Validate` and `Admin approval` checks satisfied within ~30s (PR-title workflow + the auto-approval workflow), `gh pr merge --auto` would fire, GitHub would see all *required* checks green, and the PR would squash-merge before `ci.yml` even started running. Pre-consolidation runs (PR #90 etc.) didn't expose this because GitHub's auto-merge waits for in-flight checks as a courtesy, and the multi-job CI was producing checks before auto-merge fired — but with one collapsed `CI` check that didn't start until later, the courtesy wait disappeared. Fix: added `CI` to the ruleset's required-checks (now `Admin approval` / `CI` / `Validate`) via the `gh api PUT /repos/Wave-RF/WaveHouse/rulesets/15353356` round-trip, and updated both bot-clean lists to look for `CI` + `Validate`. AGENTS.md note about the required-check set updated to match. Diagnosis steps recorded inline as code comments in both workflow files so future-me knows where to look if the check name changes again. ### Added diff --git a/README.md b/README.md index b94672f7..c64dab4b 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ CI + + Go Coverage + Go Version diff --git a/docs/src/content/docs/development.md b/docs/src/content/docs/development.md index 212e0930..a87b4c7a 100644 --- a/docs/src/content/docs/development.md +++ b/docs/src/content/docs/development.md @@ -27,7 +27,7 @@ Run `make tools` once after cloning to populate everything that doesn't have to - **`golangci-lint` v2.11.4** → installed to `.bin/_/` (version-pinned in the Makefile; bumping the version triggers a reinstall). Not in `go.mod` because its dependency tree conflicts with the main module. - **`air` v1.65.1** → installed to `.bin/_/` via `go install`; used by `make dev` for hot-reload. Same exclusion principle as `golangci-lint` — air's transitive deps (Hugo, Sass libs) would bloat `go.sum`. -- **Go `tool` deps** (`gotestsum`, `gofumpt`, `goimports`, `govulncheck`, `go-test-coverage`, `deadcode`, `gsa`, `goda`) — pinned in `go.mod` via native `tool` directives (Go 1.24+), invoked with `go tool `. `make tools` runs `go mod download` so they're cached; they compile lazily on first invocation. +- **Go `tool` deps** (`gotestsum`, `gofumpt`, `goimports`, `govulncheck`, `go-test-coverage`, `gocover-cobertura`, `deadcode`, `gsa`, `goda`) — pinned in `go.mod` via native `tool` directives (Go 1.24+), invoked with `go tool `. `make tools` runs `go mod download` so they're cached; they compile lazily on first invocation. - **pnpm deps** for `clients/ts/`, `tests/e2e/sdk/`, and `docs/` (via `pnpm install --frozen-lockfile`). `make tools` runs only the pnpm install; the Playwright Chromium binary (~130 MB) is fetched on-demand by `make build-docs` / `make dev-docs` via the internal `install-playwright-docs` target, so Go-only contributors don't pay the download cost. When you do hit `build-docs` / `dev-docs`, Chromium is required by `rehype-mermaid` (SVG diagram rendering at build time; nothing else in the docs *build* uses a browser — the manual `docs/scripts/screenshot.mjs` QA helper drives the same Chromium). `starlight-links-validator` runs under `build-docs` / CI only — the `dev-docs` watch loop skips it so a mid-edit dangling link doesn't fail every rebuild (CI still enforces link validity before merge; run `DOCS_WATCH_STRICT=1 make dev-docs` to keep the validator on locally). The `--with-deps` flag (which apt-installs Chromium's system libraries: `libnspr4`, `libnss3`, etc.) is only added when `$CI` is set, so contributor laptops don't get an unexpected `sudo` prompt. On Linux dev machines without those libs already present, run `pnpm exec playwright install-deps chromium` once manually. The docs site is a pnpm workspace package (`wavehouse-docs`); the root Makefile drives it directly via `pnpm --filter` (no sub-Makefile) — the `*-docs` targets show up in `make help`. It is also a real `@wavehouse/sdk` consumer (the landing page's live demo imports the workspace package), so `check-docs` / `build-docs` / `dev-docs` build the SDK first via `build-ts`; if you drive Astro directly through pnpm (e.g. `pnpm --filter wavehouse-docs run start`), run `make build-ts` once first so the dep resolves. ### Verify your setup diff --git a/go.mod b/go.mod index 041a5d2b..b87ff894 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.4 tool ( github.com/Zxilly/go-size-analyzer/cmd/gsa + github.com/boumenot/gocover-cobertura github.com/loov/goda github.com/vladopajic/go-test-coverage/v2 golang.org/x/tools/cmd/deadcode @@ -74,6 +75,7 @@ require ( github.com/blacktop/go-dwarf v1.0.14 // indirect github.com/blacktop/go-macho v1.1.271 // indirect github.com/bluele/gcache v0.0.2 // indirect + github.com/boumenot/gocover-cobertura v1.5.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect diff --git a/go.sum b/go.sum index e578f52d..08df4bfb 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ github.com/blacktop/go-macho v1.1.271 h1:KpC13blu4m1LVUv3WPU3a73u1xNFapDeA4UJp/J github.com/blacktop/go-macho v1.1.271/go.mod h1:Hc5E2Lvt/U1VT+jOxr1O5l/LNFJeMYK4eAmDfazTiGc= github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw= github.com/bluele/gcache v0.0.2/go.mod h1:m15KV+ECjptwSPxKhOhQoAFQVtUFjTVkc3H8o0t/fp0= +github.com/boumenot/gocover-cobertura v1.5.0 h1:S2eXZ5snlTl+IGLXiM0litlpy9gf8AU8NagMaxX3nZM= +github.com/boumenot/gocover-cobertura v1.5.0/go.mod h1:iB1/+oDwfRlsDzABskkid0cNdQ1A+u3O91XUJZWqgtg= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= diff --git a/scripts/ci/publish-badge.sh b/scripts/ci/publish-badge.sh new file mode 100755 index 00000000..d82e8d5a --- /dev/null +++ b/scripts/ci/publish-badge.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# Publish a shields.io endpoint JSON to the orphan `badges` branch, which the +# README coverage badge reads over raw.githubusercontent.com (unrestricted now +# the repo is public — #133). +# +# Runs ONLY in the main-push `badge` job — the sole holder of contents:write — +# off a trusted-main checkout, so a push to this branch can never be influenced +# by PR code (.github/workflows/README.md, invariant 5). +# +# `badges` is a detached, single-purpose store with no shared history to +# protect: we fast-forward a tiny commit onto it (creating it as an orphan the +# first time), and no-op when the number hasn't changed. The workflow-level +# concurrency group serializes main-push runs, so the push never races itself. +# +# Usage: scripts/ci/publish-badge.sh +# Requires a checkout with push credentials (actions/checkout default). + +set -euo pipefail + +src="${1:?usage: publish-badge.sh }" +dest="${2:?usage: publish-badge.sh }" +branch="badges" + +[ -f "$src" ] || { echo "::error::badge source not found: $src" >&2; exit 1; } + +# A linked worktree keeps the job's main checkout untouched. +work="$(mktemp -d)" +trap 'git worktree remove --force "$work" 2>/dev/null || true' EXIT + +if git fetch --depth=1 origin "$branch" 2>/dev/null; then + git worktree add --force "$work" "origin/$branch" + git -C "$work" checkout -B "$branch" +else + echo "badges branch not found — creating it." + git worktree add --force --detach "$work" + git -C "$work" checkout --orphan "$branch" + git -C "$work" rm -rf . >/dev/null 2>&1 || true +fi + +mkdir -p "$work/$(dirname "$dest")" +cp "$src" "$work/$dest" +git -C "$work" add "$dest" + +if git -C "$work" diff --cached --quiet; then + echo "Coverage badge unchanged — nothing to publish." + exit 0 +fi + +git -C "$work" \ + -c user.name="github-actions[bot]" \ + -c user.email="41898282+github-actions[bot]@users.noreply.github.com" \ + commit -q -m "chore(badge): update coverage badge" +git -C "$work" push origin "$branch" +echo "Published $dest to the $branch branch." diff --git a/scripts/cov/main.go b/scripts/cov/main.go index dc1c6424..f4250bfa 100644 --- a/scripts/cov/main.go +++ b/scripts/cov/main.go @@ -21,6 +21,10 @@ // cov threshold Print the configured threshold for // (or "total"). Used by the SDK pipeline to // pass into vitest's --coverage.thresholds. +// cov badge Emit the shields.io endpoint JSON for the merged +// Go-total coverage (the number threshold.total +// gates) to stdout — CI publishes it to the +// `badges` branch for the README coverage badge. // // All thresholds come from .testcoverage.yml — `threshold.total` is the // canonical merged-coverage gate (the same field go-test-coverage reads @@ -30,6 +34,7 @@ package main import ( "context" + "encoding/json" "fmt" "os" "os/exec" @@ -142,6 +147,10 @@ func main() { if err := report(cfg); err != nil { fatal("%v", err) } + case "badge": + if err := badge(cfg); err != nil { + fatal("%v", err) + } case "threshold": if len(os.Args) < 3 { usage() @@ -153,7 +162,7 @@ func main() { } func usage() { - fmt.Fprintln(os.Stderr, "usage: cov render | merge | ts-merge | merge-all | report | threshold ") + fmt.Fprintln(os.Stderr, "usage: cov render | merge | ts-merge | merge-all | report | badge | threshold ") os.Exit(2) } @@ -614,6 +623,65 @@ func printReport(rows []reportRow) { fmt.Println() } +// badgeData is the shields.io endpoint schema (https://shields.io/endpoint). +// The README's coverage badge is an pointing at img.shields.io/endpoint +// whose url= is this JSON, published to the `badges` branch by CI. Emitting it +// here means the badge always shows the exact number `make cov` gated. +type badgeData struct { + SchemaVersion int `json:"schemaVersion"` + Label string `json:"label"` + Message string `json:"message"` + Color string `json:"color"` +} + +// badge writes the shields.io endpoint JSON for the merged Go-total coverage +// (global excludes only — the same number threshold.total gates) to stdout, +// and nothing else, so the caller can redirect it straight to a file. It reads +// the profile `make cov`/`cov report` already rendered to tmp/coverage/total. +func badge(c *config) error { + profile := filepath.Join(root, "total", "coverage.txt") + if _, err := os.Stat(profile); err != nil { + return fmt.Errorf("no merged Go profile at %s — run `make cov` first", profile) + } + _, total, covered, err := parseCoverage(profile, c, c.excludesFor("")) + if err != nil { + return err + } + msg, color := "unknown", "lightgrey" + if total > 0 { + pct := float64(covered) * 100.0 / float64(total) + msg = fmt.Sprintf("%.1f%%", pct) + color = badgeColor(pct, c.Threshold.Total) + } + out, err := json.Marshal(badgeData{SchemaVersion: 1, Label: "coverage", Message: msg, Color: color}) + if err != nil { + return err + } + fmt.Println(string(out)) + return nil +} + +// badgeColor maps a coverage percentage to a shields.io color anchored on the +// configured gate: at/above the gate reads green, warming through yellow to +// red below it (so the badge color tracks the same line the build enforces). +func badgeColor(pct float64, gate int) string { + g := float64(gate) + switch { + case pct >= g+10: + return "brightgreen" + case pct >= g: + return "green" + case pct >= g-10: + return "yellowgreen" + case pct >= g-20: + return "yellow" + case pct >= g-30: + return "orange" + default: + return "red" + } +} + // formatPctBare is formatPct without the trailing % ("85.1"), or "n/a". func formatPctBare(covered, total int) string { if total == 0 {