[cristim]

Refs #525 (PR 2 of 3; PR 3 keeps cancel/recover for pending rows, cancel-then-DB-failure retry, empty AWS status and the detached ctx for the release).

PR1 (#604) derived the ClientToken from the request. That left two gaps, both reproduced on current main through HandleRequest with a fake AWS that dedups on ClientToken (3 failing tests on main, run before this change):

• Lost AWS response, then a retry after a month boundary: the default schedule changes with the remaining months, so the token changes and AWS creates a second listing (2 listings on main, TestMarketplaceListHTTP_RetryAcrossMonthBoundaryDoesNotDuplicate).
• Crash between create and persist (persist fails, compensating cancel fails, recording fails): the row stays pending and every retry gets 409 (..._CrashBetweenCreateAndPersistIsReconciled). The stale-row case: row.ListingID is read before the claim, so a request holding a stale row replays a canceled listing and gets a 502 (..._StaleRowAfterCancelGetsNewListing).

Change:

• Migration 000105 adds purchase_history.listing_client_token (TEXT) and listing_price_schedule (JSONB), both nullable (up and down; integration test added). Next free number: 000104 is the highest on main and no open PR touches migrations.
• ClaimMarketplaceListingSlot writes a random token and the schedule in the same conditional UPDATE that claims the slot, before the AWS call, and returns the claimed row's listing id, prior state, token and schedule (RETURNING). The handler uses only that, so a stale pre-claim row is never consulted. The derived-token function is removed.
• A retry resends the stored token and stored schedule (AWS dedups; no IdempotentParameterMismatch). The response shows the schedule actually used.
• A pending row that holds a token can be claimed again (the same token makes a concurrent or crashed attempt safe to resume). On such a resumed claim the handler first calls DescribeReservedInstancesListings, matches ClientToken, and records the listing if AWS has it (dead listing: record it, 502, next attempt is fresh); if AWS has nothing it creates with the persisted token.
• UpdatePurchaseHistoryListing (any recorded listing) clears the token and schedule; a new ReleaseMarketplaceListingClaim restores the prior state and keeps the attempt after an ambiguous error, drops it after a definitive client fault (400, AWS created nothing).

Decisions the issue left open:

1. Reconcile runs inline on the next list request, not in a background poller. Cancel does not reconcile pending rows (PR 3).
2. Token is random and persisted, not derived; schedule persisted as JSON next to it. A caller-supplied schedule on a resumed attempt is ignored in favor of the stored one.
3. Pending rows with a token are claimable again; legacy pending rows without a token stay stuck until PR 3.
4. The reconcile lookup is a small adapter over the AWS SDK in this repo (FindMarketplaceListingByToken), because the ec2 service wrapper in cloud-commitments-go has no lookup by token; no contract change there.
5. The ambiguous vs definitive split is the existing 400/502 mapping of mapAWSMarketplaceError.

Tests: handler tests drive HandleRequest against an in-memory row with the store's claim/release/record semantics and a fake AWS (dedup on token, lost responses, parameter mismatch). Mutations that fail them: token not persisted before the AWS call (and persisted after), token re-derived on retry, schedule re-derived on retry, reconcile skipped, release always dropping the attempt. The claim/release SQL has a Postgres integration test (CI; also run locally against a throwaway Postgres 17 together with the 000105 up/down test, both passing). Gates: go test -race -short ./..., go vet ./..., go vet -tags integration ./internal/..., errcheck -blank, shadow, unparam, gocyclo, misspell -locale US on the touched code.

Review follow-up (second commit):

• F1 race: two racing claims made the second one read had_token and the prior state from a stale snapshot, so it skipped the reconcile and sent its own schedule with the first request's token. The claim now locks the row first (FOR UPDATE in a CTE) and derives Resumed from the returned row (its token differs from the one this claim tried to write). Integration test checkConcurrentMarketplaceClaim holds a first claim in an open transaction, blocks a second claim on the row lock, commits, and asserts a resumed claim with the first token and schedule. Run against a throwaway Postgres 17: passes; with the old snapshot behavior it fails.
• F3 fencing: ReleaseMarketplaceListingClaim takes the token and only touches a pending row that holds it. A stale drop or keep release is a no-op (checkMarketplaceReleaseIsFenced; the mutant without the token condition fails it).
• F4: a resumed claim releases to an empty state and keeps the recorded listing id. List, cancel and claim treat that the same as the original canceled or closed state, so it is harmless; documented on MarketplaceListingClaim.
• Pending takeover risk: two requests in flight with the same token can both reach AWS. Both use the stored schedule and count, so the parameters are identical and AWS dedups on the token.
• Stored schedule wins; the schedule used is returned as price_schedule in the list response (marketplace endpoints are not in openapi.yaml, so nothing to update there).
• Removed PR1 tests and their replacements: AmbiguousAWSErrorRetryDoesNotDuplicateListing -> TestMarketplaceListHTTP_RetryAcrossMonthBoundaryDoesNotDuplicate and ..._RetryReusesPersistedTokenAndSchedule; RetryAfterRolledBackPersistFailureGetsNewListing and ReturnedCanceledStateIsAnError -> same-named TestMarketplaceListHTTP_*; TestMarketplaceClientToken and its replaying fake have no equivalent because the derivation function is gone (the fake AWS now checks the token and schedule are on the row before the call).
• Lint: errcheck in the mock, rangeValCopy and gosec G117 fixed.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 18 billable files and costs up to $4.50.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 23 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 14a0c121-dc40-45fa-aacb-a771123e9624

📥 Commits

Reviewing files that changed from the base of the PR and between fa10886 and c67bbb5.

📒 Files selected for processing (18)

• internal/analytics/collector_test.go
• internal/api/handler.go
• internal/api/handler_marketplace.go
• internal/api/handler_marketplace_attempt.go
• internal/api/handler_marketplace_attempt_test.go
• internal/api/handler_marketplace_credentials_test.go
• internal/api/handler_marketplace_test.go
• internal/api/handler_purchase_action_test.go
• internal/api/nullable_purchase_cost_test.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_marketplace_claim_integration_test.go
• internal/config/types.go
• internal/database/postgres/migrations/000105_purchase_history_listing_attempt.down.sql
• internal/database/postgres/migrations/000105_purchase_history_listing_attempt.up.sql
• internal/database/postgres/migrations/000105_purchase_history_listing_attempt_test.go
• internal/mocks/stores.go
• internal/server/test_helpers_test.go

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}