Feature/bidding support clean#131
Conversation
|
@pharwasz is attempting to deploy a commit to the Cankat's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Pls check for approval thnx |
There was a problem hiding this comment.
Review ÔÇö changes requested
The bidding schema/API extension makes sense, but:
- CI failing ÔÇö rebase onto
main - Migration number conflict:
0009_add_bidding_support.sqlmay clash with other0008/0009migrations on main (e.g. dividends PR #129) - Negotiation statuses: Issue asks for
negotiating/accepted/counter_offerÔÇö diff only showsbidPricecolumn; status enums may be missing - No tests
Please rebase, renumber migrations, and add tests.
enliven17
left a comment
There was a problem hiding this comment.
Review — changes requested
The bidding schema/API extension makes sense, but:
- CI failing — rebase onto
main - Migration number conflict:
0009_add_bidding_support.sqlmay clash with other0008/0009migrations on main (e.g. dividends PR #129) - Negotiation statuses: Issue asks for
negotiating/accepted/counter_offer— diff only showsbidPricecolumn; status enums may be missing - No tests
Please rebase, renumber migrations, and add tests.
enliven17
left a comment
There was a problem hiding this comment.
I can't merge this bidding support as-is. Invalid bid payloads are silently ignored because submitBidSchema.safeParse(requestBody) falls back to { bidPrice: undefined, status: undefined }, so a malformed bid request can still purchase at the default service price instead of returning 400. The PR also allows clients to set job status directly, including terminal states like completed, which is unsafe for a payment/fulfillment flow. Please split bidding from purchase fulfillment, reject invalid bid payloads explicitly, and only let the server transition statuses after verification/settlement.
What about this? |
Alright Will get it done |
|
Hi @enliven17, I've addressed all the feedback from your last two reviews. Here's a breakdown of what was fixed: 1. Invalid bid payloads now explicitly rejected (400) 2. Client status submissions locked down 3. Migration renumbered to 4. Rebased onto main 5. Tests added All changes are in commit |
- Add bidPrice column to tlsCommerceJobs schema - Add status enum for negotiation states (negotiating, accepted, counter_offer) - Create migration file 0008_add_bidding_support.sql - Update Zod schemas with submitBidSchema for bid validation - Modify /api/talos/[id]/service POST endpoint to support bid payloads - Payment verification uses bidPrice when provided, falls back to service price - Job creation stores bidPrice and custom status when provided
e82fd6c to
3b6fe9d
Compare
enliven17
left a comment
There was a problem hiding this comment.
Thanks for the update. This is closer, but I still can't merge it yet.
Blocking issues:
route.diffis committed as a binary file. This looks accidental and should be removed from the PR.- The service route still lets the requester-provided
bidPricebecome theexpectedAmountforverifyX402Payment. That means a client can lower the amount they need to pay just by sending a lower bid in the same request. A bid needs a server-side accepted/approved state before it can replaceservice.pricefor payment verification and revenue/job amounts. - The new e2e assertions such as
expect(res.status).not.toBe(400)are too broad; they can pass while the endpoint is still failing for the wrong reason. Please add focused tests around the validation/schema behavior and the accepted-bid payment path. - Please clean up the indentation/formatting in the modified route before the next review pass.
Once the accidental file is removed and bid pricing is tied to a server-accepted negotiation state, I can re-check it.
I've addressed all 4 blocking issues from your latest review: 1. 2. 3. Tests tightened 4. Indentation/formatting cleaned up All changes are in commit |
enliven17
left a comment
There was a problem hiding this comment.
Thanks for the update. The accidental route.diff file is gone and verifying x402 against service.price fixes the main payment-bypass issue. However, this still is not merge-ready.
Blocking issue: after verifying the customer paid the full service price, the route still records amount: bidData.bidPrice ?? service.price in both tlsCommerceJobs and tlsRevenues. That means a request with bidPrice: 1 can pay the listed price but persist the job/revenue as 1, which under-reports revenue and creates inconsistent financial history.
Please keep amount and revenue records tied to the actually verified payment amount (service.price) unless there is a separate server-side accepted bid/negotiation record. Store bidPrice only in the separate nullable bidPrice column for negotiation metadata.
Local check: targeted ESLint passed with only an existing unused beforeAll warning in tests/api-e2e.test.ts.
…tion metadata only, remove unused beforeAll import
enliven17
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. I rechecked this on top of the current main: the accidental file is gone, x402 verification stays tied to the listed service price, and job/revenue �mount now stays tied to service.price while �idPrice is stored only as negotiation metadata. Targeted ESLint and the OpenAPI snapshot test passed locally. Note:
ext build still fails on an existing unrelated buy-token FeeBumpTransaction type issue in main, not on this PR's changed files.
Closes #63
Feature: Update Commerce Schema and API Endpoints for Bidding Support
Summary
This PR introduces bidding and price negotiation support for commerce jobs by extending the database schema, validation layer, and service marketplace API endpoints. The implementation enables users to submit custom bid amounts and track negotiation status instead of relying solely on fixed pricing.
Changes Made
Database Updates
Added bidPrice column to tlsCommerceJobs
Added support for negotiation statuses:
negotiating
accepted
counter_offer
Added database migration:
web/drizzle/0008_add_bidding_support.sql
Validation Updates
Updated Zod schemas to validate bid-related payloads
Added bid price validation for positive numeric values
Added status validation for supported negotiation states
API Updates
Updated /api/talos/[id]/service endpoints to support bid submissions
Added bid payload validation
Stored bid information during job creation
Updated payment verification flow to use bidPrice when provided, with fallback to the original service price
Acceptance Criteria Checklist
Add bidPrice column to tlsCommerceJobs
Support negotiation statuses (negotiating, accepted, counter_offer)
Update Zod schemas for bid payload validation
Modify /api/talos/[id]/service endpoints to support bidding functionality
Testing
Verified bid payload validation
Verified job creation with bid values
Verified negotiation status handling
Verified payment calculation uses bid price when available