Commit489a5ac
fix(billing): connect AI metering to the gate that was reading a dead column
fix(billing): connect AI metering to the gate that was reading a dead column assertAiQuota is already called at the top of ai-review, ai-review-trio and ai-ci-healer, and it reads userQuotas.aiTokensUsedThisMonth. Nothing ever incremented that column: bumpUsage, whose entire job is to increment it, had ZERO callers anywhere in the codebase. So the counter sat at 0 forever, `allowed` was always true, and the monthly AI cap could never trip — unbounded per-user spend against a paid API. The lead said "AI quota never metered". That is wrong, and the truth is more specific: token counts were being recorded accurately the whole time, via recordAiCost into ai_cost_events, which the enforcement path does not read. Metering and enforcement were wired to two different stores. Every part existed — bumpUsage, invalidateQuotaCache, checkAiQuotaCached, assertAiQuota — and none were connected. recordAiCost is the one funnel every AI call site already passes through and it already carries the owner and the token counts, so metering goes there. It runs before the ledger insert and independently of it: a failed dashboard row must not skip metering. Never throws, like the insert it accompanies. Calls with no owner are skipped — platform-internal work has nobody to bill and must not land on whichever user is nearby. The cached verdict is invalidated after each bump, or a burst would keep reusing a stale allow and sail past the cap. Second half, and it matters as much: getUserQuota does NOT roll the monthly cycle, and resetIfCycleExpired's only other caller is the repo-creation gate. Making the counter live without also rolling it would have permanently locked users out of AI once they hit the cap, released only by the unrelated act of creating a repository — a worse bug than the one being fixed. checkAiQuotaCached now rolls the cycle before reading, behind the existing 60s cache so it costs at most one query per user per minute. It still fails open. Fault injection earned its keep twice here. Replacing the call with `void meterAiQuota;` did NOT fail the first version of the test, because asserting the identifier appears is not asserting it is invoked — the assertion now requires the awaited call. Removing the cycle reset fails 1. Suite: 3492 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 files changed+268−0489a5ac1ed61408988f705194187b3b6fad2dc9f
3 changed files+268−0
Addedsrc/__tests__/ai-quota-metering.test.ts+187−0View fileUnifiedSplit
@@ -0,0 +1,187 @@
1/**
2 * Regression guard: the AI spend cap must actually be able to trip.
3 *
4 * assertAiQuota is called at the top of the AI features (ai-review,
5 * ai-review-trio, ai-ci-healer). It reads
6 * userQuotas.aiTokensUsedThisMonth — and NOTHING ever incremented that
7 * column. bumpUsage, the function whose entire job is to increment it, had
8 * zero callers anywhere in the codebase. So the counter sat at 0 forever,
9 * `allowed` was always true, and the monthly AI cap could never fire.
10 *
11 * Token counts were being recorded accurately the whole time, just into
12 * ai_cost_events via recordAiCost, which the enforcement path does not read.
13 * Metering and enforcement were wired to two different stores. Every piece
14 * existed — bumpUsage, invalidateQuotaCache, checkAiQuotaCached,
15 * assertAiQuota — and none of them were connected.
16 *
17 * The second half matters as much as the first: getUserQuota does NOT roll
18 * the monthly cycle, and resetIfCycleExpired's only other caller is the
19 * repo-creation gate. Making the counter live without also rolling it would
20 * have permanently locked users out of AI once they hit the cap, released
21 * only by the unrelated act of creating a repository. That would be a worse
22 * bug than the one being fixed.
23 *
24 * The rule is asserted directly; the wiring is asserted from source, because
25 * the defect was an absent call and no test of the rule alone would catch a
26 * regression that simply stops calling it.
27 */
28
29import { describe, it, expect } from "bun:test";
30import { readFileSync } from "node:fs";
31import { join } from "node:path";
32import { quotaChargeForCall } from "../lib/ai-cost-tracker";
33
34const USER = "11111111-1111-1111-1111-111111111111";
35
36describe("quotaChargeForCall", () => {
37 it("charges the owner for input plus output tokens", () => {
38 expect(
39 quotaChargeForCall({ ownerUserId: USER, inputTokens: 900, outputTokens: 100 })
40 ).toEqual({ userId: USER, tokens: 1000 });
41 });
42
43 it("charges nobody when the call has no owner", () => {
44 // Platform-internal work (cron sweeps, system tasks) has nobody to bill
45 // and must not be charged to whichever user happens to be nearby.
46 expect(
47 quotaChargeForCall({ ownerUserId: null, inputTokens: 500, outputTokens: 500 })
48 ).toBeNull();
49 expect(
50 quotaChargeForCall({ ownerUserId: "", inputTokens: 500, outputTokens: 500 })
51 ).toBeNull();
52 expect(
53 quotaChargeForCall({ inputTokens: 500, outputTokens: 500 })
54 ).toBeNull();
55 });
56
57 it("skips calls that consumed nothing", () => {
58 expect(
59 quotaChargeForCall({ ownerUserId: USER, inputTokens: 0, outputTokens: 0 })
60 ).toBeNull();
61 });
62
63 it("never charges a negative or fractional amount", () => {
64 // A bad usage payload must not be able to REFUND quota and hand a user
65 // unlimited spend.
66 expect(
67 quotaChargeForCall({ ownerUserId: USER, inputTokens: -5000, outputTokens: 10 })
68 ).toEqual({ userId: USER, tokens: 10 });
69 expect(
70 quotaChargeForCall({ ownerUserId: USER, inputTokens: -50, outputTokens: -50 })
71 ).toBeNull();
72 expect(
73 quotaChargeForCall({ ownerUserId: USER, inputTokens: 1.9, outputTokens: 0.9 })
74 ).toEqual({ userId: USER, tokens: 1 });
75 });
76
77 it("tolerates missing token fields", () => {
78 expect(
79 quotaChargeForCall({
80 ownerUserId: USER,
81 inputTokens: undefined as unknown as number,
82 outputTokens: 7,
83 })
84 ).toEqual({ userId: USER, tokens: 7 });
85 });
86});
87
88// --- the wiring must stay connected ----------------------------------------
89//
90// Strip ONLY line comments: a block-comment regex eats path-like text.
91
92function source(...rel: string[]): string {
93 const raw = readFileSync(join(import.meta.dir, "..", ...rel), "utf8");
94 const stripped = raw
95 .split("\n")
96 .filter((l) => !l.trim().startsWith("//") && !l.trim().startsWith("*"))
97 .join("\n");
98 expect(stripped.length).toBeGreaterThan(0);
99 return stripped;
100}
101
102/** Slice by anchor, never by character count. */
103function slice(src: string, startAnchor: string, endAnchor: string): string {
104 const start = src.indexOf(startAnchor);
105 expect(start).toBeGreaterThan(-1);
106 const end = src.indexOf(endAnchor, start + startAnchor.length);
107 expect(end).toBeGreaterThan(start);
108 const body = src.slice(start, end);
109 expect(body.length).toBeGreaterThan(0);
110 return body;
111}
112
113describe("recordAiCost meters the quota", () => {
114 const src = source("lib", "ai-cost-tracker.ts");
115
116 it("awaits the meter — referencing it is not calling it", () => {
117 // Asserting the identifier merely APPEARS is too weak: `void
118 // meterAiQuota;` satisfies that while metering nothing. Require the
119 // awaited invocation.
120 const body = slice(
121 src,
122 "export async function recordAiCost",
123 "export async function summarize"
124 );
125 expect(body).toContain("await meterAiQuota(args)");
126 });
127
128 it("meters before the ledger insert, so a ledger failure cannot skip it", () => {
129 const body = slice(
130 src,
131 "export async function recordAiCost",
132 "export async function summarize"
133 );
134 expect(body.indexOf("await meterAiQuota(args)")).toBeLessThan(
135 body.indexOf("insert(aiCostEvents)")
136 );
137 });
138
139 it("bumps the column the enforcement path actually reads", () => {
140 expect(src).toContain("bumpUsage");
141 expect(src).toContain("aiTokensUsedThisMonth");
142 });
143
144 it("invalidates the cached verdict after bumping", () => {
145 // Otherwise a burst of calls keeps reusing a stale allow until the cache
146 // expires and sails past the cap.
147 const meter = slice(src, "async function meterAiQuota", "export interface");
148 expect(meter).toContain("invalidateQuotaCache");
149 expect(meter.indexOf("bumpUsage")).toBeLessThan(
150 meter.indexOf("invalidateQuotaCache")
151 );
152 });
153});
154
155describe("the quota gate rolls the monthly cycle", () => {
156 const src = source("lib", "billing.ts");
157
158 it("resets an expired cycle before reading usage", () => {
159 const body = slice(
160 src,
161 "export async function checkAiQuotaCached",
162 "export function invalidateQuotaCache"
163 );
164 expect(body).toContain("resetIfCycleExpired");
165 expect(body.indexOf("resetIfCycleExpired")).toBeLessThan(
166 body.indexOf("getUserQuota(userId)")
167 );
168 });
169
170 it("still fails open on billing errors", () => {
171 // Billing must never be a hard dependency of the primary path.
172 const body = slice(
173 src,
174 "export async function checkAiQuotaCached",
175 "export function invalidateQuotaCache"
176 );
177 expect(body).toContain("allowed: true");
178 });
179});
180
181describe("AI entry points still assert the quota", () => {
182 for (const file of ["ai-review.ts", "ai-review-trio.ts", "ai-ci-healer.ts"]) {
183 it(`${file} calls assertAiQuota`, () => {
184 expect(source("lib", file)).toContain("assertAiQuota(");
185 });
186 }
187});
Modifiedsrc/lib/ai-cost-tracker.ts+73−0View fileUnifiedSplit
@@ -121,6 +121,62 @@ export function computeCentsForCall(
121121 return Math.max(1, Math.ceil(cents));
122122}
123123
124/**
125 * Decide who to charge for a call and how many tokens, or null for nothing.
126 *
127 * Split out from the database work so the rule is testable on its own.
128 * Returns null when there is no owner — platform-internal calls (cron
129 * sweeps, system tasks) have nobody to bill and must not land on whichever
130 * user happens to be nearby — and when the call consumed no tokens.
131 */
132export function quotaChargeForCall(args: {
133 ownerUserId?: string | null;
134 inputTokens: number;
135 outputTokens: number;
136}): { userId: string; tokens: number } | null {
137 const userId = args.ownerUserId ?? "";
138 if (!userId) return null;
139
140 const tokens =
141 Math.max(0, Math.floor(args.inputTokens || 0)) +
142 Math.max(0, Math.floor(args.outputTokens || 0));
143 if (tokens <= 0) return null;
144
145 return { userId, tokens };
146}
147
148/**
149 * Add this call's tokens to the owner's monthly quota counter.
150 *
151 * Never throws — quota accounting must not be able to break an AI feature,
152 * exactly like the ledger insert it accompanies.
153 *
154 * Skipped when there is no owner: platform-internal calls (cron sweeps,
155 * system tasks) have nobody to bill and must not be charged to whichever
156 * user happens to be nearby.
157 */
158async function meterAiQuota(args: RecordAiCostArgs): Promise<void> {
159 const charge = quotaChargeForCall(args);
160 if (!charge) return;
161 const { userId, tokens } = charge;
162
163 try {
164 const { bumpUsage, invalidateQuotaCache } = await import("./billing");
165 await bumpUsage(userId, "aiTokensUsedThisMonth", tokens);
166 // Drop the cached verdict so the next call re-reads. Without this the
167 // gate would keep using a stale allow for up to the cache TTL, letting a
168 // burst of calls run well past the cap before it bites.
169 invalidateQuotaCache(userId);
170 } catch (err) {
171 if (process.env.DEBUG_AI_COST === "1") {
172 console.warn(
173 "[ai-cost-tracker] quota meter failed:",
174 err instanceof Error ? err.message : err
175 );
176 }
177 }
178}
179
124180export interface RecordAiCostArgs {
125181 ownerUserId?: string | null;
126182 repositoryId?: string | null;
@@ -138,6 +194,23 @@ export interface RecordAiCostArgs {
138194 * level and swallowed so the calling AI feature continues unaffected.
139195 */
140196export async function recordAiCost(args: RecordAiCostArgs): Promise<void> {
197 // Meter the spend against the user's monthly AI token quota.
198 //
199 // This is the wire that was missing. assertAiQuota is already called at the
200 // top of the AI features, and it reads userQuotas.aiTokensUsedThisMonth —
201 // but NOTHING incremented that column, so it sat at 0 forever, the gate
202 // always allowed, and the cap could never trip. Token counts were being
203 // recorded faithfully, just into ai_cost_events, which the enforcement path
204 // does not read. Metering and enforcement were wired to different stores.
205 //
206 // recordAiCost is the one funnel every AI call site already goes through
207 // and it already carries the owner and the token counts, so this is the
208 // single correct place to close the loop.
209 //
210 // Deliberately independent of the ledger insert below: a failure to write
211 // the observational dashboard row must not skip metering, and vice versa.
212 await meterAiQuota(args);
213
141214 try {
142215 const cents = computeCentsForCall(
143216 args.model,
Modifiedsrc/lib/billing.ts+8−0View fileUnifiedSplit
@@ -382,6 +382,14 @@ export async function checkAiQuotaCached(
382382 }
383383
384384 try {
385 // Roll the monthly counters over before reading them. getUserQuota does
386 // NOT do this, and the only other caller of resetIfCycleExpired is the
387 // repo-creation gate — so without this line a user who hit their AI cap
388 // would stay blocked indefinitely, and would be released only by the
389 // unrelated act of creating a repository. This read is behind the 60s
390 // quota cache, so the check costs at most one query per user per minute.
391 await resetIfCycleExpired(userId).catch(() => false);
392
385393 const quota = await getUserQuota(userId);
386394 const limit = quota.plan.aiTokensMonthly;
387395 const used = quota.usage.aiTokensUsedThisMonth;
388396