feat: add marketplace credits gateway skeleton#326
Open
zozo123 wants to merge 11 commits into
Open
Conversation
Contributor
|
Codex review: needs real behavior proof before merge. Reviewed June 14, 2026, 4:30 AM ET / 08:30 UTC. Summary Reproducibility: yes. for the review findings: source inspection of the latest PR head shows the CLI max-credits omission, invalid JSON 500 path, and product-specific docs. Runtime behavior proof from a real coordinator setup is still missing. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Approve the provider-neutral marketplace contract in #282 first, then land a preview slice with neutral docs, strict CLI/API validation, compatibility expectations, focused tests, and redacted real coordinator proof. Do we have a high-confidence way to reproduce the issue? Yes for the review findings: source inspection of the latest PR head shows the CLI max-credits omission, invalid JSON 500 path, and product-specific docs. Runtime behavior proof from a real coordinator setup is still missing. Is this the best way to solve the issue? No. The skeleton is useful design signal, but the public marketplace contract needs maintainer approval before merge, and the validation, docs, and proof blockers should be resolved first. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ccc27374948c. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
Contributor
Author
|
Autoreview (codex) + improvements + push to finish line for #326 Ran the autoreview helper per skill (from agent-skills copy, --mode branch --base origin/main on the PR checkout of feat/payment-marketplace-skeleton). Review runs & findings (all verified by reading the exact code in worker/src/marketplace.ts and adjacent):
All reported findings fixed in this stack of commits on the branch (before the credits failure):
Pushed the fix commits (f29a462..26c450a on the fork branch, which updates this PR head). The skeleton is improved with the routing/cost/input contract hardened while keeping the preview surfaces and "one credit card, smart routing" model. The last engine run was blocked by credits before confirming 0 findings, but every finding from successful runs was read, reproduced in the source, and fixed at the right (marketplace.ts) surface with no over-scope refactors. Focused Go tests re-ran clean. If credits allow, a post-push autoreview on the new tip would be the final clean signal. Otherwise this is the improved state ready for review/merge + the publish step. (Also see prior comment for the broader agentic/Airbyte vision requirements as review tasks.) |
…k unavailable for unsupported provider/target; ceil fractional TTLs (autoreview P2 findings) - quoteProviders now intersects user-requested providers against status.supportedProviders (from CRABBOX_MARKETPLACE_ALLOWED_PROVIDERS) before unique/return; rejects with invalid_provider if none allowed. - quoteCandidate now checks providerSupportsTarget (linux-only for hetzner/gcp etc. per existing backends); sets unavailableReason and available=false. - positiveInt for ttlSeconds now uses ceil+max(1,...) so 0.5->1, never 0 credits from raw JSON (CLI path unaffected). - Added small providerSupportsTarget helper next to quoteTarget/quoteStrategy for scope. - Go tests still green; no other surfaces touched. These close the contract violations for routing policy and compatible candidates in the quote API.
…explicit macOS/windows rules) per lease compat (autoreview P2 sibling)
…); validate providers array shape to avoid 500 on malformed JSON (autoreview P3s) - Distinguish undefined ttlSeconds (use default) from supplied invalid (<=0, non-finite, string etc.): throw MarketplaceInputError 'invalid_ttl' for the latter so clients get 400 instead of silent wrong-duration quote. - Early guard in marketplaceQuote: if providers supplied and not array, or body not object, throw proper MarketplaceInputError (caught as 400) instead of letting .length/.filter throw TypeError (500). - These close the input-trust issues for the quote endpoint while keeping the happy path unchanged.
… nonEmpty (prevents .trim crash on number/object JSON; autoreview P3 input validation)
…s name) as serverType for leaseCost base fallback (accurate rate-card path for documented --class beast remains; addresses autoreview P2 on class pricing)
…side the allowlist (no silent drop); reject supplied but invalid maxCredits (P3 input validation from final autoreview)
… Worker CI format Two changes, both within the no-billing skeleton boundary: 1. CI fix: run oxfmt on src/marketplace.ts so the Worker `format:check` step passes (the only failing check on the PR). 2. One step ahead: implement the weighted same-priority load balancing the PR already documents but left as a dead `weight` field. Adds a `weighted` routing strategy that ranks candidates within the winning priority tier by `weight`, and a per-candidate `routeShare` (0..1) previewing how traffic would split across equivalent providers (weights 3:1 -> share 75%/25%). Priority still selects the failover tier; weight load-balances inside it. Preview-only: routes no traffic, captures no payment, moves no credits. Surfaced through worker (marketplace.ts), coordinator client + CLI printer (`share=NN%`), with worker + Go tests and updated command/feature docs. Verification: worker format:check / lint / check / check:node / build, full worker tests (482), go test ./internal/cli marketplace tests, gofmt, check-command-docs.mjs, git diff --check — all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e rounding (self-review follow-up) Driven by an adversarial self-review of the weighted-routing commit. Verified findings fixed, plus the next in-scope step. Still no billing/real routing. One step ahead — routingPlan: - The quote now returns an explicit failover ladder under the weighted strategy: an ordered array of priority tiers (highest first), each with its available members (provider, routeKey, weight, routeShare) and an `active` flag on the tier that serves the selected candidate. Makes the documented routing-groups contract (priority failover + weighted load balancing) reviewable as data instead of implicit in candidate sort order. Preview-only: routes nothing. - Single-source computation: buildRoutingPlan() is the one place tiers/shares are derived; the flat candidate.routeShare is mirrored from the active tier so the two views can never diverge. Verified fixes from the review: - routeShare now uses largest-remainder allocation, so per-tier shares sum to exactly 1 (independent 4dp rounding drifted, e.g. 1:1:1 -> 0.9999). The CLI renders integer percents with largest-remainder too, so a tier always reads 100% (e.g. 34%/33%/33%). - Removed the dead `weight > 0 ? : 0` guards (weight is always >= 1). - Quote ID now folds in strategy + maxCredits so distinct requests don't collide. - Docs: clarified selection is deterministic (heaviest available, not probabilistic); documented routingPlan in command + feature docs; ticked roadmap item 5. Tests (worker 484 total; go cli marketplace 4): - equal-weight (1:1:1) shares sum to exactly 1; maxCredits-excluded candidate dropped from the weighted tier + denominator; strategy-distinct quote IDs; parameterized non-weighted strategies leave routeShare/routingPlan unset; CLI negative `share=` guard for cheapest; CLI routing-plan ladder render (largest-remainder display totals 100%, active + failover tiers). Verification: worker format:check/lint/check/check:node/build + full tests (484), go test ./internal/cli marketplace tests, go vet, gofmt, check-command-docs.mjs, git diff --check — all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…te + routing paths Tighten the disabled-by-default credits gateway skeleton without adding any real money movement: - worker: reject non-string provider/target/strategy and non-string providers[] members up front with clear MarketplaceInputError codes (previously a numeric provider produced a misleading invalid_provider via the allowlist filter, and a non-string target/strategy fell through to the union check). - worker tests: cover the new shape-validation codes, the non-object request guard, the explicit-provider-outside-allowlist rejection, and a guard that enabling the gateway never flips payments/ledger/enforcement on. - worker fleet tests: cover the HTTP 409 disabled path and the HTTP 400 malformed-input path for /v1/marketplace/quotes. - cli tests: add direct unit coverage for parseMarketplaceTTL, marketplaceRoutePercents (largest-remainder per-tier 100% allocation), and marketplaceShareSuffix (weighted-share/failover rendering). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zozo123
force-pushed
the
feat/payment-marketplace-skeleton
branch
from
f4112ed to
d6c41c5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /v1/marketplace/statusandPOST /v1/marketplace/quoteson the coordinatorcrabbox marketplace statusandcrabbox marketplace quotefor gateway visibility and smart-routing quote previewspriority,weight, active/enabled) inspired by AI gateway failover/load-balancing patternscrabbox.sh), and self-hosted/BYOK payment-operator modes so billing/legal/support ownership stays deployment-configuredNotes
This is intentionally not a live billing implementation. It does not capture payment, mutate a credit ledger, reserve credits, enforce credits during lease creation, or settle providers. The PR makes those boundaries explicit so maintainers can review the contract before real-money code lands.
Closes #282
Verification
npm ci --prefix workernpm run format:check --prefix workernpm run check --prefix workernpm run lint --prefix workernpm test --prefix worker -- marketplace.test.ts fleet.test.tsgo test ./internal/clinode scripts/check-command-docs.mjsgit diff --check