fix: unwedge CI, and three real defects the broken test harness was hiding #4040

Originally written by @ccantynz-alt on GitHub.
Imported from https://github.com/ccantynz-alt/AlecRae.com/pull/91

Description

A bug-finding pass that started by running the gates instead of trusting the docs. The first finding was that CI was red on main for both required status checks, so no PR could merge at all — bun run lint failed on two pre-existing errors and bun run test had 32 failures. The branch protection added in issue #106 to stop main breaking had instead wedged the repo shut; the only reason nobody hit it is that enforce_admins is deliberately off (#66), so admin direct-pushes kept landing.

Root-causing the test failures turned up the bug underneath, and that bug was hiding two more.

#121 — @alecrae/ai-engine's export map pointed every subpath's import condition at a dist/ tree that is never built (the package's build script is a bare echo). Bun resolves the bun condition, so production and typecheck were clean — but vitest resolves import, so 20 tests died at module load. Among them: hard quota enforcement, suppression-list enforcement at send time, and POST /v1/messages/send validation. Three real protections whose tests had not executed for as long as that map has been in this shape.

For the audit trail: issue #29 recorded this as "a pre-existing, unrelated vitest↔@alecrae/ai-engine dist-resolution gap" and wrote off 6 voice-message tests as unrunnable in the sandbox. That diagnosis was wrong — it was the export map, and all 6 now pass.

#122 — monthly email quota was silently unenforced whenever Redis was unavailable. checkQuota()'s DB fallback counts events rows of type email.queued, and nothing in the codebase ever inserted one — the only reference to that string anywhere was quota.ts's own SELECT. The fallback counted 0 forever, so allowed was unconditionally true and the 429 reported "0 sent". incrementQuota() early-returned on no-Redis behind the comment "DB counter is updated separately by the existing code", describing code that does not exist. Not a corner case: getRedis() returns null until ioredis's async ready fires, so the first send after every API restart took this path, and per CLAUDE.md's own infra history the box ran with no redis-server installed at all until 2026-07-14.

#123 — plan-tier resolution failed open to PAID tiers in three places, inverting what plan-gate.ts (#86) exists to do:

• a token with no tier claim became "starter" and passed requirePlan("personal");
• if the accounts lookup threw, a production API key was handed "pro" outright — a transient Postgres blip unlocked all 60 requirePlan("pro") mounts (every Claude-backed endpoint) with no spend ceiling;
• the refresh path defaulted to "starter" while every login/register/OAuth path already defaulted to "free", so a user's tier could silently move upward just by refreshing a token.

Also rewrote the three vapron test files, which still asserted the guessed tRPC transport that #83 replaced — they weren't merely failing, they encoded known-wrong behaviour as the expectation.

Reported, deliberately not fixed (#124)

Each needs a decision that isn't mine to make unilaterally:

• Rate limiting is trivially bypassable. getClientIp() reads CF-Connecting-IP first with no trusted-proxy verification anywhere in the repo. Any client reaching the API other than through Cloudflare sets it to a fresh random value per request and gets a fresh bucket, defeating the 10/min brute-force cap on /v1/auth/* and compounding #117. Fixing it requires knowing which proxy is authoritative and whether the origin is reachable off-Cloudflare — getting it wrong breaks real IP attribution.
• middleware/idempotency() has no in-flight lock, only a post-hoc response cache, and it's mounted on POST /v1/messages/send. Two concurrent requests with the same Idempotency-Key both send for real. Sequential retries (the documented case) do work.

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactor (no functional changes)
• Documentation update
• Infrastructure / CI change

Testing

Gates, all from a clean bun install:

21 new tests. The two behavioural fixes were verified to genuinely catch their bug — I reverted each fix and confirmed the tests fail against the old code, then restored:

• tests/quota-db-fallback.test.ts (8) — runs quota.ts with Redis pointed at a closed port. 3 fail pre-fix. The load-bearing one asserts the writer and reader agree on the exact event-type string, since the defect was the two sides disagreeing, not either being wrong alone. Noting honestly: the other 5 document the limit math but do not discriminate pre/post-fix.

• tests/tier-fail-closed.test.ts (13) — asserts the fail-closed contract at the tier→plan-gate boundary, and pins the DB-vs-API spelling bridge ("professional" → "pro"), since regressing that 403s every paying Pro customer on every Pro-gated route — the opposite failure mode.

• tests/vapron.test.ts rewritten (15) — covers both transports separately so they can't be swapped again silently.

• Unit tests added/updated

• Integration tests added/updated

• Manual testing performed

One thing I could not verify and am flagging rather than glossing: the email.queued row is a new write on the send path. It's fire-and-forget and tested against a mocked DB, but it has not run against live Postgres.

Checklist

• My code follows the project style guidelines
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing tests pass locally (bun run test)
• Lint and type-check pass (bun run lint && bun run typecheck)
• I have updated documentation where necessary (bun run docs:check passes — 62 docs)

Breaking changes

None to any public API. Two behaviour changes worth calling out for review:

1. Email quota now actually enforces during a Redis outage. Previously any account could send without limit on that path. If an account has already sent past its plan limit this month, it will now correctly receive a 429 where before it would not — intended, but it is a live behaviour change, and the counter starts from event rows written after this deploy.
2. Callers whose tier can't be resolved now get "free" instead of a paid tier. Deliberately fail-closed: worst case is a paying customer briefly seeing an upgrade prompt; worst case of the old direction was uncapped AI spend on a free account. Normal resolution paths are unaffected — "professional" → "pro" still works and is now covered by a test.

No migration required. email.queued already exists in the email_event_type enum.

Screenshots

N/A

Generated by Claude Code