Skip to content

Commit ddfd5fb

Browse files
committed
fix(credits): close the five accounting holes from review
- Scope the balance's spend aggregation to provider "nodetool": BYOK predictions (fal_ai, anthropic, ...) no longer drain credits, and historical BYOK spend no longer zeroes new balances. Regression test covers mixed managed/BYOK rows. - Gate the no-payment top-up behind NODETOOL_ENABLE_TEST_TOPUP (off by default): the mutation throws FORBIDDEN otherwise, status reports testTopupEnabled, and the UI hides the button. - Direct generate_media now admits against a unit-price estimate, reserves it for the duration of the call, and records a prediction row (max of delegate-tracked cost and the unit estimate) so the balance decrements; transcribe_audio records delegate-tracked cost likewise. - Workflow runs reserve their nodetool estimate when admitted and release at the terminal state or cancel-while-queued, so concurrent or queued submissions can't multiply a small balance (process-local, TTL- bounded; a multi-instance deployment moves this into the DB like application-budgets). - NodetoolProvider builds a fresh delegate per call instead of sharing a cached instance, so absorbing cost off the delegate's cumulative counter can't double-count overlapping calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GwuPTt5T1sLkmpwWUt2N2
1 parent 1e6ce2f commit ddfd5fb

9 files changed

Lines changed: 272 additions & 65 deletions

File tree

packages/models/src/credits.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
* would write `topup` rows and flip `plan_id`; until then plan switches are
1313
* instant and top-ups are stubbed.
1414
*/
15-
import { eq, sql } from "drizzle-orm";
15+
import { and, eq, sql } from "drizzle-orm";
16+
import { NODETOOL_PROVIDER_ID } from "@nodetool-ai/protocol";
1617

1718
import { getDb } from "./db.js";
1819
import { creditLedger, userSubscriptions } from "./schema/credits.js";
@@ -222,10 +223,18 @@ export async function creditStatus(userId: string): Promise<CreditStatus> {
222223
.where(eq(creditLedger.user_id, userId));
223224
const grantedCredits = Number(grantRows[0]?.total ?? 0);
224225

226+
// Only the managed provider's spend counts against credits — BYOK
227+
// predictions (fal_ai, anthropic, …) ride the user's own keys and must
228+
// never drain the balance.
225229
const spendRows = await db
226230
.select({ total: sql<number>`COALESCE(SUM(${predictions.cost}), 0)` })
227231
.from(predictions)
228-
.where(eq(predictions.user_id, userId));
232+
.where(
233+
and(
234+
eq(predictions.user_id, userId),
235+
eq(predictions.provider, NODETOOL_PROVIDER_ID)
236+
)
237+
);
229238
const spentUsd = Number(spendRows[0]?.total ?? 0);
230239
const spentCredits = Math.ceil(spentUsd / USD_PER_CREDIT);
231240

packages/models/tests/credits.test.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ const USER = "u1";
1818
const FREE = CREDIT_PLANS.find((p) => p.id === "free")!;
1919
const CREATOR = CREDIT_PLANS.find((p) => p.id === "creator")!;
2020

21-
const spend = (cost: number) =>
21+
const spend = (cost: number, provider = "nodetool") =>
2222
Prediction.create<Prediction>({
2323
user_id: USER,
2424
node_id: "n",
2525
node_type: "test",
26-
provider: "test",
26+
provider,
2727
model: "test",
2828
cost
2929
});
@@ -99,6 +99,19 @@ describe("credits", () => {
9999
}
100100
});
101101

102+
it("only managed-provider spend counts — BYOK predictions never drain credits", async () => {
103+
await creditStatus(USER);
104+
await spend(0.5, "fal_ai");
105+
await spend(1.25, "anthropic");
106+
const untouched = await creditStatus(USER);
107+
expect(untouched.spentCredits).toBe(0);
108+
expect(untouched.balanceCredits).toBe(FREE.monthlyCredits);
109+
110+
await spend(0.02, "nodetool");
111+
const managed = await creditStatus(USER);
112+
expect(managed.spentCredits).toBe(2);
113+
});
114+
102115
it("credits are per user", async () => {
103116
await creditStatus(USER);
104117
await creditStatus("u2");

packages/protocol/src/api-schemas/credits.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export const creditStatusOutput = z.object({
2525
* are never gated — both models coexist on one server.
2626
*/
2727
meteredProvider: z.string(),
28+
/** True only on dev servers that opt into the no-payment test top-up. */
29+
testTopupEnabled: z.boolean(),
2830
plans: z.array(creditPlan)
2931
});
3032
export type CreditStatusOutput = z.infer<typeof creditStatusOutput>;

packages/runtime/src/providers/nodetool-provider.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ const PLATFORM_KEYS: Record<string, string> = {
3939

4040
export class NodetoolProvider extends BaseProvider {
4141
private secrets: Record<string, unknown>;
42-
private inner = new Map<string, BaseProvider>();
4342
private _absorbedCost = 0;
4443

4544
static override requiredSecrets(): string[] {
@@ -61,7 +60,14 @@ export class NodetoolProvider extends BaseProvider {
6160
return this.platformKey(delegateProvider) != null;
6261
}
6362

64-
/** The delegate serving a nodetool model id, constructed on platform keys. */
63+
/**
64+
* The delegate serving a nodetool model id, constructed on platform keys.
65+
* A fresh instance per call on purpose: cost is read off the delegate's
66+
* cumulative counter after the call, and a shared delegate serving
67+
* overlapping calls would double-count one call's cost into another's
68+
* absorption window. Construction is cheap — both delegates build their
69+
* network clients lazily.
70+
*/
6571
private delegateFor(modelId: string): {
6672
provider: BaseProvider;
6773
model: string;
@@ -77,15 +83,11 @@ export class NodetoolProvider extends BaseProvider {
7783
`(missing ${PLATFORM_KEYS[def.delegate.provider]}).`
7884
);
7985
}
80-
let instance = this.inner.get(def.delegate.provider);
81-
if (!instance) {
82-
instance =
83-
def.delegate.provider === "fal_ai"
84-
? new FalProvider({ FAL_API_KEY: key })
85-
: new AnthropicProvider({ ANTHROPIC_API_KEY: key });
86-
this.inner.set(def.delegate.provider, instance);
87-
}
88-
return { provider: instance, model: def.delegate.model };
86+
const provider =
87+
def.delegate.provider === "fal_ai"
88+
? new FalProvider({ FAL_API_KEY: key })
89+
: new AnthropicProvider({ ANTHROPIC_API_KEY: key });
90+
return { provider, model: def.delegate.model };
8991
}
9092

9193
/** Run a delegated call and absorb the delegate's cost delta as our own. */
@@ -111,7 +113,6 @@ export class NodetoolProvider extends BaseProvider {
111113

112114
override resetCost(): void {
113115
this._absorbedCost = 0;
114-
for (const inner of this.inner.values()) inner.resetCost();
115116
}
116117

117118
protected override declaredCapabilities(): ProviderCapability[] {

packages/websocket/src/credit-gate.ts

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,75 @@ export type CreditGateResult =
1515
| { allowed: true }
1616
| { allowed: false; reason: string };
1717

18+
/**
19+
* In-flight reservations: spend admitted but not yet in the prediction
20+
* ledger. Without them, N concurrent submissions all pass against the same
21+
* balance and a small balance authorizes N× its worth of platform spend.
22+
* A reservation is taken when a run is admitted and released at its terminal
23+
* state (or cancel-while-queued); the TTL bounds leaks from paths that never
24+
* reach either (process-local state — a multi-instance deployment needs the
25+
* reservation moved into the database, as application-budgets does).
26+
*/
27+
interface SpendReservation {
28+
usd: number;
29+
at: number;
30+
}
31+
32+
const RESERVATION_TTL_MS = 2 * 60 * 60 * 1000;
33+
const reservations = new Map<string, Map<string, SpendReservation>>();
34+
35+
export function reserveSpend(
36+
userId: string,
37+
key: string,
38+
estimatedUsd: number
39+
): void {
40+
let byKey = reservations.get(userId);
41+
if (!byKey) {
42+
byKey = new Map();
43+
reservations.set(userId, byKey);
44+
}
45+
byKey.set(key, { usd: Math.max(0, estimatedUsd), at: Date.now() });
46+
}
47+
48+
/** Releasing an unknown key is a no-op, so terminal paths can call it blindly. */
49+
export function releaseSpend(userId: string, key: string): void {
50+
const byKey = reservations.get(userId);
51+
if (!byKey) return;
52+
byKey.delete(key);
53+
if (byKey.size === 0) reservations.delete(userId);
54+
}
55+
56+
export function reservedSpendUsd(userId: string): number {
57+
const byKey = reservations.get(userId);
58+
if (!byKey) return 0;
59+
const cutoff = Date.now() - RESERVATION_TTL_MS;
60+
let total = 0;
61+
for (const [key, reservation] of byKey) {
62+
if (reservation.at < cutoff) {
63+
byKey.delete(key);
64+
continue;
65+
}
66+
total += reservation.usd;
67+
}
68+
if (byKey.size === 0) reservations.delete(userId);
69+
return total;
70+
}
71+
1872
/**
1973
* Decide whether `userId` may spend an estimated `estimatedUsd` through the
20-
* managed provider. Estimates are floors (unpriceable work estimates 0), so
21-
* an empty balance blocks even a 0-estimate call.
74+
* managed provider, counting spend already admitted but not yet settled.
75+
* Estimates are floors (unpriceable work estimates 0), so an empty balance
76+
* blocks even a 0-estimate call.
2277
*/
2378
export async function admitSpend(
2479
userId: string | null | undefined,
2580
estimatedUsd: number
2681
): Promise<CreditGateResult> {
2782
try {
28-
const decision = await checkCredits(userId ?? "1", estimatedUsd);
83+
const decision = await checkCredits(
84+
userId ?? "1",
85+
estimatedUsd + reservedSpendUsd(userId ?? "1")
86+
);
2987
return decision.allowed
3088
? { allowed: true }
3189
: { allowed: false, reason: decision.reason };

packages/websocket/src/trpc/routers/credits.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,22 @@ import {
1515
} from "@nodetool-ai/protocol/api-schemas/credits.js";
1616
import { NODETOOL_PROVIDER_ID } from "@nodetool-ai/protocol";
1717

18+
/**
19+
* Whether the unauthenticated-by-payment test top-up is allowed. Off unless
20+
* the operator explicitly opts a development server in: minted credits unlock
21+
* spend on platform-owned keys, so an open mint endpoint is an open wallet.
22+
*/
23+
const testTopupEnabled = (): boolean => {
24+
const value = process.env.NODETOOL_ENABLE_TEST_TOPUP?.toLowerCase();
25+
return value === "1" || value === "true";
26+
};
27+
1828
const statusFor = async (userId: string) => {
1929
const status = await creditStatus(userId);
2030
return {
2131
...status,
2232
meteredProvider: NODETOOL_PROVIDER_ID,
33+
testTopupEnabled: testTopupEnabled(),
2334
plans: [...CREDIT_PLANS]
2435
};
2536
};
@@ -49,14 +60,23 @@ export const creditsRouter = router({
4960
}),
5061

5162
/**
52-
* Prototype top-up: adds credits with no payment behind it. A payment
63+
* Development-only top-up: adds credits with no payment behind it, and is
64+
* refused unless the operator sets NODETOOL_ENABLE_TEST_TOPUP. A payment
5365
* provider integration replaces this mutation with a checkout session and
5466
* writes the ledger row from the webhook instead.
5567
*/
5668
topup: protectedProcedure
5769
.input(topupInput)
5870
.output(creditStatusOutput)
5971
.mutation(async ({ ctx, input }) => {
72+
if (!testTopupEnabled()) {
73+
throw new TRPCError({
74+
code: "FORBIDDEN",
75+
message:
76+
"Top-ups are disabled on this server: no payment provider is " +
77+
"configured and NODETOOL_ENABLE_TEST_TOPUP is not set."
78+
});
79+
}
6080
await grantCredits(
6181
ctx.userId,
6282
input.credits,

0 commit comments

Comments
 (0)