diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 78c06efa..4141b9dd 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -19,3 +19,4 @@ For detailed project context, architecture, code conventions, and common tasks, 5. **Verify before finishing** — before completing any task, search docs for identifiers you touched (field names, env vars, endpoints, struct names) and fix any stale references. 6. **Every new function must have tests** — use table-driven tests, shared mocks from `internal/testutil/`, and aim for 80%+ coverage. Run `make lint` and `make test` before considering work complete. 7. **Use testutil helpers** — use `MockPublisher`, `MockCache`, `MockDeduplicator`, `MockSubscriber` from `testutil/mocks.go` instead of creating ad-hoc mocks. Use `testutil.MakeJWT()` for auth tests. Use `policy.NewMemoryStore(p)` for policy tests. Use `pipes.NewMemoryStore(queries...)` for pipes tests. +8. **Validate locally before pushing** — run `make ci` (or at minimum `make coverage`) in your sandbox and confirm it passes before opening or updating a PR. The repo runs on a shared self-hosted runner pool with finite throughput and bills AI-reviewer credits per push; a speculative commit to "see what CI says" costs real minutes and dollars. If a test passes locally but flakes on CI, investigate the runner environment before patching the test — see the "Local-First Validation" section in [AGENTS.md](../AGENTS.md) for full guidance. diff --git a/.github/workflows/admin-approval.yml b/.github/workflows/admin-approval.yml index 8706d7bb..7f6ce155 100644 --- a/.github/workflows/admin-approval.yml +++ b/.github/workflows/admin-approval.yml @@ -32,7 +32,7 @@ concurrency: jobs: admin-approval: name: Admin approval - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - name: Check for admin approval env: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ef6fc57..b5d08735 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,13 @@ on: push: branches: [main] pull_request: + # Default `pull_request` types are [opened, synchronize, reopened] + # which do NOT include `ready_for_review`. Without it, flipping a + # PR from draft → ready doesn't re-trigger CI, so the e2e job + # (gated on `draft == false`) silently skips on the existing run. + # Adding `ready_for_review` ensures the full suite runs at the + # moment the PR becomes review-ready. + types: [opened, synchronize, reopened, ready_for_review] branches: [main] permissions: @@ -15,7 +22,7 @@ permissions: jobs: check: name: Check - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -34,7 +41,7 @@ jobs: lint: name: Lint - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -54,7 +61,7 @@ jobs: test: name: Test - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -83,7 +90,7 @@ jobs: build: name: Build - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -96,7 +103,7 @@ jobs: integration: name: Integration Tests - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] needs: [test] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -113,7 +120,7 @@ jobs: # Vitest unit tests for the TypeScript SDK. Fast (~30s) and runs on # every PR — no path filter so a deps bump or workflow tweak still # exercises the suite. - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -146,7 +153,7 @@ jobs: # Node to 22 which provides it — but keeping the flag while the # raw-SQL race is still unfixed). Drop once follow-up lands. if: github.event_name == 'push' || github.event.pull_request.draft == false - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] needs: [build] continue-on-error: true steps: @@ -158,5 +165,20 @@ jobs: cache: "npm" cache-dependency-path: tests/sdk/package-lock.json + # Build the wavehouse image once via the shared remote BuildKit + # and `--load` it into the runner pod's Docker daemon (DinD + # sidecar) so the docker compose stack started by setup.ts uses + # the prebuilt image instead of re-building from source. The + # BUILDX_BUILDER env var is preset on the runner pod, so buildx + # transparently routes to the shared builder + cache. + - name: Build wavehouse image (remote BuildKit, --load) + run: docker buildx build --load -t wavehouse:e2e -f deployments/Dockerfile . + - name: Run E2E tests + env: + # Setting WAVEHOUSE_IMAGE makes tests/compose.yaml use the + # prebuilt image (loaded by the previous step) instead of + # invoking its own `build:` directive. See header comment in + # tests/compose.yaml. + WAVEHOUSE_IMAGE: wavehouse:e2e run: make test-e2e diff --git a/.github/workflows/claude-agent.yml b/.github/workflows/claude-agent.yml index cd90a2a3..8583ecf4 100644 --- a/.github/workflows/claude-agent.yml +++ b/.github/workflows/claude-agent.yml @@ -36,7 +36,7 @@ concurrency: jobs: claude: name: Run Claude - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] # Gate: must be a trusted actor AND either mention @claude or apply # the `agent` label. The author_association path differs per event # so each branch is explicit. diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index cd15e902..35d9f573 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -37,7 +37,7 @@ jobs: github.event.pull_request.draft == false && github.actor != 'dependabot[bot]' && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR","CONTRIBUTOR"]'), github.event.pull_request.author_association) - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] permissions: contents: read pull-requests: write diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index 59c02085..7e052b95 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -29,7 +29,7 @@ permissions: jobs: auto-merge: if: github.actor == 'dependabot[bot]' - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - name: Fetch Dependabot metadata id: meta diff --git a/.github/workflows/label.yml b/.github/workflows/label.yml index 38d49a08..7720437a 100644 --- a/.github/workflows/label.yml +++ b/.github/workflows/label.yml @@ -26,7 +26,7 @@ concurrency: jobs: label: name: Apply labels - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/labeler@634933edcd8ababfe52f92936142cc22ac488b1b # v6.0.1 with: diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml index fdffb4ef..b2fcdbda 100644 --- a/.github/workflows/pr-title.yml +++ b/.github/workflows/pr-title.yml @@ -34,7 +34,7 @@ concurrency: jobs: validate: name: Validate - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - name: Check Conventional Commits format id: check diff --git a/.github/workflows/project-orchestrator.yml b/.github/workflows/project-orchestrator.yml index 8c367b28..bf5415d2 100644 --- a/.github/workflows/project-orchestrator.yml +++ b/.github/workflows/project-orchestrator.yml @@ -94,7 +94,7 @@ env: jobs: orchestrate: - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - name: Guard — require PROJECT_BOARD_TOKEN env: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0cf675cd..8d76d81d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,7 +12,7 @@ permissions: jobs: release: name: Release - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml index 5f710b23..46a126ff 100644 --- a/.github/workflows/triage.yml +++ b/.github/workflows/triage.yml @@ -26,7 +26,7 @@ concurrency: jobs: classify: name: Classify and label - runs-on: ubuntu-latest + runs-on: [wave-rf-runners] # Skip re-triggering on our own label edits. if: github.actor != 'github-actions[bot]' && github.event.issue.pull_request == null steps: diff --git a/AGENTS.md b/AGENTS.md index 85888622..b40b05b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,37 @@ Dev tools (`gotestsum`, `gofumpt`, `goimports`) are pinned in `go.mod` via nativ - **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/sdk/` exercise the full pipeline (ingest → ClickHouse → query) and simultaneously validate backend behavior and SDK correctness. Use `make test-e2e` to run, or `make test-e2e-dev` for watch mode. Add new E2E scenarios as `tests/sdk/*.test.ts` files using helpers from `tests/sdk/helpers.ts`. +## Local-First Validation (MANDATORY) + +**Validate locally before pushing. Do not use CI as your first feedback loop.** The repo runs on a shared 4-runner self-hosted VM with finite throughput and bills AI-reviewer (Claude, Gemini, Copilot) credits on every push. A speculative "let's see what CI says" commit costs real minutes and real dollars and is visible to the entire team as churn. Every push should represent a change you have locally verified to pass the same gates CI will run. + +### Before every push + +Run the CI-equivalent locally: + +```bash +make ci # Full parity with CI: tidy + fmt + lint + vulncheck + build + tests +make coverage # Matches the CI `Test` job (race detector; -p 1 in CI for package serialization) +``` + +If `make ci` passes, your commit has crossed the same gates CI will run. If it fails, fix it before pushing — don't rely on CI to surface issues that took seconds to catch locally. + +For workflow-only changes where `make ci` isn't relevant, manually read through your YAML diff line-by-line before pushing. If you already have `actionlint` installed locally, also run `actionlint .github/workflows/*.yml`; CI's own billing makes "push and see" for workflow-file iteration especially wasteful. + +### If local passes but CI fails + +**Treat this as an environment mismatch, not a test bug, until proven otherwise.** Tests that pass on a dev machine in milliseconds but time out on the self-hosted VM point to runner-side problems (I/O pressure, zombie processes, disk contention, shared-VM fsync storms) — not to flaky test code. Investigate the runner before changing tests or production code. Masking environment issues with longer timeouts or retries tends to compound: today's 5s bump becomes tomorrow's 30s bump becomes next week's unbounded wait, and the underlying runner problem keeps slowly degrading. + +Order of operations before patching tests for "CI flakiness": + +1. **Reproduce the reported failure locally first.** `go test -race -run TestFoo ./...` on your machine. If it fails locally, you have a real test bug; fix it with deterministic primitives (use `c.Wait()` not `time.Sleep`, use `require.Eventually` not `time.Sleep` then assert, use channel sync not goroutine scheduling assumptions). +2. **If it passes locally, try to reproduce under load.** Run 4 concurrent copies of `make coverage` to simulate the VM's shared-runner contention. If that still passes, the problem is the VM — not the test. +3. **Only then touch the runner.** SSH in, check `iostat -x 2`, `pgrep -af nats-server`, `df -h`, `du -sh /opt/github/action-runner-*/_work`. Environment fixes (cleanup crons, tmpfs for test temp dirs, slower runner count, faster disk) stay scoped to the runner and don't pollute the codebase. + +### When delegating to another agent + +If you hand work to a subagent or another Claude session, tell them explicitly: *"Run locally first. Do not push to CI until `make ci` passes on your checkout."* Agents default to "commit and let CI run" because it looks like progress; in this repo that default is expensive. Override it at delegation time. + ## Review Response (MANDATORY) **Every review comment on a PR gets a substantive reply, and every conversation gets resolved before merge. This applies equally to human reviewers and AI reviewers (Copilot, Gemini Code Assist, claude-review, future bots). The `main branch protection` ruleset enforces `required_review_thread_resolution: true`, so unresolved threads literally block merge.** diff --git a/internal/cache/local_test.go b/internal/cache/local_test.go index cce96b62..137a74c5 100644 --- a/internal/cache/local_test.go +++ b/internal/cache/local_test.go @@ -67,10 +67,10 @@ func TestLocalCache_Overwrite(t *testing.T) { defer func() { _ = c.Close() }() ctx := context.Background() - _ = c.Set(ctx, "key", []byte("v1"), 10*time.Second) - time.Sleep(10 * time.Millisecond) - _ = c.Set(ctx, "key", []byte("v2"), 10*time.Second) - time.Sleep(10 * time.Millisecond) + require.NoError(t, c.Set(ctx, "key", []byte("v1"), 10*time.Second)) + c.Wait() + require.NoError(t, c.Set(ctx, "key", []byte("v2"), 10*time.Second)) + c.Wait() val, _, err := c.Get(ctx, "key") assert.NoError(t, err)