Skip to content

fix: restore ci test integrity and close audit findings#111

Merged
AKogut merged 1 commit into
mainfrom
fix/ci-test-integrity
Jul 18, 2026
Merged

fix: restore ci test integrity and close audit findings#111
AKogut merged 1 commit into
mainfrom
fix/ci-test-integrity

Conversation

@AKogut

@AKogut AKogut commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Closes #22

The important part: CI was green but hollow

A re-audit found that 31 database-backed tests were silently skipping in CI. Turbo v2 runs tasks in strict env mode, and turbo.json declared no env allowlist, so the job-level DATABASE_URL never reached vitest — hasDb was false and every describe.skipIf quietly skipped. The real CI logs showed db 4/7, api 20/23 and worker 7/11 skipped. Every green check on the recent PRs verified only the non-database paths.

Fixes:

  • turbo.json declares env: ["DATABASE_URL", "REQUIRE_DB"] on the test task.
  • CI sets REQUIRE_DB=1, and each global setup now throws when it is set without a DATABASE_URL, so this can never silently regress again. Local runs without a database still skip gracefully.
  • CI applies the three test-schema migrations once, sequentially, before the turbo run — removing concurrent prisma migrate deploy contention on a cold database.

CI went from 91 tests actually executing to 122, with zero skips.

Defects found in the same audit

  • Rate limiter leaked memory — buckets were never evicted, growing without bound per project. Added a windowed sweep plus a size() probe and a test.
  • queue.depth() ignored in-flight jobs — it counted only pending, so backpressure under-reported the real backlog. Now counts pending + processing, with a test.
  • A COUNT(*) per ingest request when backpressure was enabled, on the path whose whole design goal is to stay fast. Depth is now cached with a TTL (test asserts three requests hit the database once).
  • Queue-depth gauge was registered per buildApp call — moved to the service entry point so it registers once.

Dead code and hygiene

  • test.retry_of was declared in the conventions but never emitted. Now emitted explicitly, with the mapper preferring it and falling back to attempt-order reconstruction.
  • Removed unused publicProcedure and paginatedSchema.
  • CONVENTIONS_VERSION now derives from CONTRACT_VERSION so the two cannot drift.
  • IngestClient clock defaulted to () => 0, making the buffer filename timestamp meaningless — now Date.now.
  • Added READMEs for the five publishable packages; @flakemetry/ai is marked private until it has an implementation, since publishing an empty package would be worse than not publishing.

Also closes #22, which was implemented in #96 but never linked.

Testing

  • Full pipeline on a cold database: 38/38 tasks, 122 tests, zero skips
  • Verified the REQUIRE_DB guard fails the run when the database is absent

Known, not fully resolved

One db queue test flaked once in roughly ten cold-database runs. The most likely source of nondeterminism (concurrent migrations) is removed, but the root cause is not proven. If it recurs, the next place to look is FOR UPDATE SKIP LOCKED behaviour under concurrent load.

@AKogut AKogut self-assigned this Jul 18, 2026
@AKogut
AKogut merged commit f2405ed into main Jul 18, 2026
2 checks passed
@AKogut
AKogut deleted the fix/ci-test-integrity branch July 18, 2026 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Query API (tRPC): runs, test detail history, flaky board, RCA

1 participant