feat(edge): hosted operator router and credit checkout functions#26
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 314cbd192e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await debit({ | ||
| userId, | ||
| amountCents: validCreditCostCents, | ||
| reason: "Operator routing", | ||
| idempotencyKey: newIdempotencyKey("router", `${userId}:${idempotencyKey}`), |
There was a problem hiding this comment.
When a caller reuses the same x-idempotency-key while still having at least creditCostCents balance, this code invokes chatComplete first and then ignores the duplicate result from debit_credits. That means the first request is charged, but every subsequent request with that key can get a fresh model proposal with no new ledger row, making hosted routing free for reused keys; reserve/record the idempotency key before the provider call or reject duplicates before returning a new proposal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 14dd1bc — proposals persist in the ledger row metadata; a replay of a completed key returns the stored proposal with ZERO extra model calls (pre-model duplicate lookup), closing the free-model-call vector; a failed first call leaves no row and retries fresh.
| if (!Array.isArray(value)) { | ||
| throw boundaryViolation(); | ||
| } | ||
|
|
There was a problem hiding this comment.
When an authenticated caller sends a very large commandText or many large context labels, this validator accepts the strings without any length or count limit and the handler later forwards JSON.stringify(routeRequest) to the model while charging a fixed credit cost. That allows a single request to consume far more provider tokens than the configured debit, or to repeatedly force context-overflow 500s; add bounded string lengths and array sizes before chatComplete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 14dd1bc — commandText capped 4000, context strings 200 each / arrays 100 items; over-limit rejected.
| /\/(?:[^/\s]+\/)+[^/\s]+/.test(value) || | ||
| /[a-z]:[\\/]/i.test(value) || | ||
| /\\\\/.test(value) || | ||
| /\b[a-z0-9_.-]+\\[a-z0-9_.-]+\\/i.test(value) || |
There was a problem hiding this comment.
For inputs such as open /etc, use ../secrets, or open foo/bar, this regex does not match because it requires at least two slash-separated components after the slash, so readRouteString accepts and forwards those local path identifiers to the hosted model. Since this guard is the boundary preventing project paths from leaving the client, include single-component absolute paths and single-slash relative paths in the rejection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 14dd1bc — single-segment and ../ traversal POSIX paths are now caught (guard table extended, sane non-path false-positive checks kept).
| } | ||
|
|
||
| async function readCheckoutRequest(req: Request): Promise<{ pack: unknown }> { | ||
| const body = await req.json(); |
There was a problem hiding this comment.
When an authenticated caller sends malformed JSON or a body-less request to create-credit-checkout, req.json() throws a SyntaxError that falls through to the generic catch and returns 500 internal_error even though the request is invalid and Stripe is never involved. Mirror the router parser by translating JSON parse failures into an EdgeSharedError/400 so clients do not treat bad checkout input as a transient server failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 14dd1bc — malformed checkout JSON returns 400.
| const supabaseAnonKey = requiredEnv("SUPABASE_ANON_KEY"); | ||
| const stripe = new Stripe(requiredEnv("STRIPE_SECRET_KEY")); | ||
|
|
||
| Deno.serve(async (req) => { |
There was a problem hiding this comment.
When the app invokes this function (and operator-router) from a browser or Tauri webview using Authorization/JSON headers, the browser sends an OPTIONS preflight; this handler falls into the auth/body path and returns no access-control-* headers, so the real checkout/router request is blocked before it reaches the function. Add an OPTIONS branch and CORS headers to every response for these app-facing functions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 14dd1bc — both functions answer the OPTIONS preflight with CORS headers via a shared helper; responses carry ACAO.
ElbertePlinio
force-pushed
the
feat/hosted-router
branch
from
314cbd1 to
14dd1bc
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14dd1bc930
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /\b(?!and\/or\b)[a-z0-9_.-]+\/[a-z0-9_.-]+\b/i.test(value) || | ||
| /[a-z]:[\\/]/i.test(value) || | ||
| /\\\\/.test(value) || | ||
| /\b[a-z0-9_.-]+\\[a-z0-9_.-]+\\/i.test(value) || |
There was a problem hiding this comment.
For a Windows relative path with only two segments like open foo\bar, none of the earlier checks match and this regex requires another trailing backslash, so assertRouteRequest accepts it and forwards the path-like identifier to the hosted model. That breaks the boundary guarantee that Windows paths are rejected before routing; make this check also match single-backslash two-segment paths.
Useful? React with 👍 / 👎.
| /\\\\/.test(value) || | ||
| /\b[a-z0-9_.-]+\\[a-z0-9_.-]+\\/i.test(value) || | ||
| /\b[a-z0-9-]+(?::\d{1,5})\b/i.test(value) || | ||
| /\b[a-z0-9-]+(?:\.[a-z0-9-]+)+\b/i.test(value) || |
There was a problem hiding this comment.
For supported createChat commands that include an exact model/version, e.g. create codex chat using gpt-5.4-mini or node 20.11, this hostname regex matches the dotted token and assertRouteRequest returns boundary_violation before routing. Since the router prompt supports a model field, this blocks legitimate model-selection commands rather than just hostnames; narrow hostname detection so model/version tokens can pass.
Useful? React with 👍 / 👎.
| return { | ||
| "access-control-allow-origin": "*", | ||
| "access-control-allow-methods": "POST, OPTIONS", | ||
| "access-control-allow-headers": "authorization, content-type, x-idempotency-key", |
There was a problem hiding this comment.
When these new browser-facing Edge Functions are invoked via supabase.functions.invoke, the request includes the project key in apikey (and commonly x-client-info) in addition to authorization; because this allow-list omits those headers, the browser preflight is rejected before either operator-router or create-credit-checkout reaches its handler. Include the Supabase client headers here along with x-idempotency-key.
Useful? React with 👍 / 👎.
|
|
||
| function containsForbiddenIdentifier(value: string): boolean { | ||
| return ( | ||
| /\b(?:https?|wss?):\/\//i.test(value) || |
There was a problem hiding this comment.
A command such as connect ssh://buildserver still passes the boundary check because the URL regex only covers HTTP(S)/WS(S), and a dotless authority is not caught by the later hostname pattern unless it has a port. That forwards a URL/host identifier to the hosted model despite the documented boundary guarantee, so the scheme check should cover generic URI schemes or explicitly reject these authorities.
Useful? React with 👍 / 👎.
| /\b[a-z0-9-]+(?::\d{1,5})\b/i.test(value) || | ||
| /\b[a-z0-9-]+(?:\.[a-z0-9-]+)+\b/i.test(value) || | ||
| /\b100\.(?:6[4-9]|[7-9]\d|1[01]\d|12[0-7])\.\d{1,3}\.\d{1,3}\b/.test(value) || |
There was a problem hiding this comment.
When the command includes connect localhost without a port or an IPv6 literal such as [fd7a:115c:a1e0::1], none of these host checks match: the dotless-host rule requires :<port>, and the tailnet rule only covers IPv4. Those host identifiers are therefore sent to the router model, so add explicit handling for bare localhost/dotless hosts that are clearly host references and IPv6 literals.
Useful? React with 👍 / 👎.
ElbertePlinio
force-pushed
the
feat/hosted-router
branch
from
14dd1bc to
eab024f
Compare
ElbertePlinio
force-pushed
the
feat/hosted-router
branch
2 times, most recently
from
f1f5caf to
7ee24f2
Compare
ElbertePlinio
force-pushed
the
feat/hosted-router
branch
from
7ee24f2 to
29c64dc
Compare
1 participant
Refs #10 · refs pickforge/pickforge#133 (the M3 hosted router). The platform half — the app lane + Buy-credits UI is the consuming PR in pickforge.
What
@pickforge/edge-sharedgainsassertRouteRequest(strict allowlist + forbidden-identifier guard) andcreateOperatorRouterHandler(pure, injectable) + a sharedoperatorRouterSystemPromptsupabase/functions/operator-router: verify JWT → boundary-guard → balance pre-check → model call → debit-after-success so a model failure never charges (a retry routes fresh, charges exactly once); model viaOPENAI_ROUTER_MODEL(default gpt-5.4-mini) over an OpenAI-compatible endpoint, returns the raw proposal for the app to validate with its own zod schema (same contract as BYO — the model gets no more trust)supabase/functions/create-credit-checkout: authenticated, packs constrained to the three server-side price ids, URLs from env — no arbitrary price/amount/URLnpm:@pickforge/*@0.7.0imports are source-consistentValidation
bun run check: 195 tests (guard bypass table, debit-after-success + model-failure-no-charge + insufficient-precheck + duplicate-charges-once, checkout price binding), coverage + builds greenNot tested: live model call — deploy after the 0.7.0 publish; the final model pin (5.4-mini vs a 5.6 tier) is the owner eval on #133 and is one env var. Follow-ups: monthly free-credit grant (cron) and variable/usage-based pricing are noted, not in this slice.