-
Notifications
You must be signed in to change notification settings - Fork 0
feat: DB-driven gateway margin + mount in core #175
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,6 +51,8 @@ export interface StripeServices { | |||||||||
|
|
||||||||||
| export interface GatewayServices { | ||||||||||
| serviceKeyRepo: IServiceKeyRepository; | ||||||||||
| meter: import("../metering/emitter.js").MeterEmitter; | ||||||||||
| budgetChecker: import("../monetization/budget/budget-checker.js").IBudgetChecker; | ||||||||||
|
Comment on lines
+54
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Rule Used: WOPR codebase conventions:
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/server/container.ts
Line: 54-55
Comment:
**`meter` typed as concrete class instead of its interface**
`IMeterEmitter` is exported from `src/metering/index.ts` and matches the convention that sub-container fields take repository/emitter *interfaces*, not implementations. `budgetChecker` correctly uses `IBudgetChecker`; `meter` should be consistent:
```suggestion
meter: import("../metering/emitter.js").IMeterEmitter;
budgetChecker: import("../monetization/budget/budget-checker.js").IBudgetChecker;
```
**Rule Used:** WOPR codebase conventions:
- Repository pattern is... ([source](https://app.greptile.com/review/custom-context?memory=158cf84c-c9a3-4390-b104-b5df2c2423d6))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||
| } | ||||||||||
|
|
||||||||||
| export interface HotPoolServices { | ||||||||||
|
|
@@ -239,8 +241,14 @@ export async function buildContainer(bootConfig: BootConfig): Promise<PlatformCo | |||||||||
| let gateway: GatewayServices | null = null; | ||||||||||
| if (bootConfig.features.gateway) { | ||||||||||
| const { DrizzleServiceKeyRepository } = await import("../gateway/service-key-repository.js"); | ||||||||||
| const { MeterEmitter } = await import("../metering/emitter.js"); | ||||||||||
| const { DrizzleMeterEventRepository } = await import("../metering/meter-event-repository.js"); | ||||||||||
| const { DrizzleBudgetChecker } = await import("../monetization/budget/budget-checker.js"); | ||||||||||
|
|
||||||||||
| const serviceKeyRepo: IServiceKeyRepository = new DrizzleServiceKeyRepository(db as never); | ||||||||||
| gateway = { serviceKeyRepo }; | ||||||||||
| const meter = new MeterEmitter(new DrizzleMeterEventRepository(db as never), { flushIntervalMs: 5_000 }); | ||||||||||
| const budgetChecker = new DrizzleBudgetChecker(db as never, { cacheTtlMs: 30_000 }); | ||||||||||
| gateway = { serviceKeyRepo, meter, budgetChecker }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // 12. Build the container (hotPool bound after construction) | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,12 +44,12 @@ export interface MountConfig { | |||||||||||||||
| * 6. Product-specific route plugins | ||||||||||||||||
| * 7. Tenant proxy middleware (catch-all — must be last) | ||||||||||||||||
| */ | ||||||||||||||||
| export function mountRoutes( | ||||||||||||||||
| export async function mountRoutes( | ||||||||||||||||
| app: Hono, | ||||||||||||||||
| container: PlatformContainer, | ||||||||||||||||
| config: MountConfig, | ||||||||||||||||
| plugins: RoutePlugin[] = [], | ||||||||||||||||
| ): void { | ||||||||||||||||
| ): Promise<void> { | ||||||||||||||||
| // 1. CORS middleware | ||||||||||||||||
| const origins = deriveCorsOrigins(container.productConfig.product, container.productConfig.domains); | ||||||||||||||||
| app.use( | ||||||||||||||||
|
|
@@ -95,7 +95,45 @@ export function mountRoutes( | |||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // 6. Product-specific route plugins | ||||||||||||||||
| // 6. Metered inference gateway (when gateway is enabled) | ||||||||||||||||
| if (container.gateway) { | ||||||||||||||||
| // Validate billing config exists in DB — fail hard, no silent defaults | ||||||||||||||||
| const billingConfig = container.productConfig.billing; | ||||||||||||||||
| const marginConfig = billingConfig?.marginConfig as { default?: number } | null; | ||||||||||||||||
| if (!marginConfig?.default) { | ||||||||||||||||
|
Comment on lines
+102
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Low The check const marginConfig = billingConfig?.marginConfig as { default?: number } | null;
- if (!marginConfig?.default) {
+ if (typeof marginConfig?.default !== 'number') {🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||||||||||||||||
| throw new Error( | ||||||||||||||||
| "Gateway enabled but product_billing_config.margin_config.default is not set. " + | ||||||||||||||||
| "Seed the DB: INSERT INTO product_billing_config (product_id, margin_config) VALUES ('<id>', '{\"default\": 4.0}')", | ||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+103
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Every WOPR codebase convention rule explicitly states: "Margin config is stored in If the intent is to permanently migrate to DB-driven margin, the custom instruction rules and Rule Used: WOPR codebase conventions:
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/server/mount-routes.ts
Line: 103-108
Comment:
**Margin config convention violated — must use `MARGIN_CONFIG_JSON` env var**
Every WOPR codebase convention rule explicitly states: *"Margin config is stored in `MARGIN_CONFIG_JSON` env var, never in source"* (with `loadMarginConfig()` in `src/monetization/adapters/margin-config.ts` as the canonical loader). This PR moves the margin to `product_billing_config.margin_config.default` in the database instead, which is an intentional but undiscussed departure from the team-wide convention.
If the intent is to permanently migrate to DB-driven margin, the custom instruction rules and `loadMarginConfig()` path should be updated in the same PR so no future author is misled. If this is a product-specific override on top of the env-var baseline, the env-var path should still be respected as a fallback.
**Rule Used:** WOPR codebase conventions:
- Repository pattern is... ([source](https://app.greptile.com/review/custom-context?memory=b986c674-3c9f-4f81-9c16-19366bd8a16f))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+103
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/server/mount-routes.ts
Line: 103-108
Comment:
**Falsy guard rejects a `0` margin**
`!marginConfig?.default` is `true` when `default` is `0`, which would throw even though `0` is a valid (if degenerate) numeric value. Prefer an explicit null/undefined check:
```suggestion
if (marginConfig?.default == null) {
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||
|
|
||||||||||||||||
| // Live margin — reads from productConfig per-request (DB-cached with TTL) | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium The 🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||||||||||||||||
| const initialMargin = marginConfig.default; | ||||||||||||||||
| const resolveMargin = (): number => { | ||||||||||||||||
| const cfg = container.productConfig.billing?.marginConfig as { default?: number } | null; | ||||||||||||||||
| return cfg?.default ?? initialMargin; | ||||||||||||||||
| }; | ||||||||||||||||
|
Comment on lines
+110
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR description claims "resolveMargin getter for runtime DB changes without restart", but since To make the margin truly live the resolver must call Prompt To Fix With AIThis is a comment left during a code review.
Path: src/server/mount-routes.ts
Line: 110-115
Comment:
**`resolveMargin` reads a stale boot-time snapshot, not live DB values**
`container.productConfig` is a plain `ProductConfig` object set once when `buildContainer` runs and is **never mutated or reassigned** afterward. The closure therefore always returns the margin captured at startup — identical to `initialMargin`. The `ProductConfigService` (with its TTL cache) is available as `container.productConfigService`, but it is not used here.
The PR description claims "resolveMargin getter for runtime DB changes without restart", but since `container.productConfig` is a frozen snapshot, this does not work. An operator who updates `margin_config.default` in the DB mid-run will see no effect until the server restarts.
To make the margin truly live the resolver must call `container.productConfigService.getBySlug(slug)` and await it; but since `resolveMargin` is typed as `() => number` (sync), this requires either making the type `() => Promise<number>` throughout the gateway, or adding a separate background-refresh loop that pushes the cached value into a mutable local variable. As-is, the "live margin" feature is a no-op.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||
|
|
||||||||||||||||
| const gw = container.gateway; | ||||||||||||||||
| const { mountGateway } = await import("../gateway/index.js"); | ||||||||||||||||
| mountGateway(app, { | ||||||||||||||||
| meter: gw.meter, | ||||||||||||||||
| budgetChecker: gw.budgetChecker, | ||||||||||||||||
| creditLedger: container.creditLedger, | ||||||||||||||||
| resolveMargin, | ||||||||||||||||
| providers: { | ||||||||||||||||
| openrouter: process.env.OPENROUTER_API_KEY | ||||||||||||||||
| ? { apiKey: process.env.OPENROUTER_API_KEY, baseUrl: process.env.OPENROUTER_BASE_URL || undefined } | ||||||||||||||||
| : undefined, | ||||||||||||||||
| }, | ||||||||||||||||
| resolveServiceKey: async (key: string) => { | ||||||||||||||||
| const tenant = await gw.serviceKeyRepo.resolve(key); | ||||||||||||||||
| return tenant ?? null; | ||||||||||||||||
| }, | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // 7. Product-specific route plugins | ||||||||||||||||
| for (const plugin of plugins) { | ||||||||||||||||
| app.route(plugin.path, plugin.handler(container)); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These four lines look like temporary markers left over from debugging the build pipeline and should not be committed to the public export barrel:
Prompt To Fix With AI