[IAmJSD]

Summary

This PR handles gap 1 of the issue and settles gap 3 by documenting it. Gap 2 (superseded listings) and the "Before exposing proceeds" checks are left open; see "Out of scope" below.

Refs #1494

Root cause

The RI Marketplace poller (#1289) reads the rows returned by ListMarketplaceListingsToPoll. That query selected only two kinds of row: active listings, and closed/cancelled listings with listed_at set and no proceeds recorded.

The list handler records a listing as pending in two cases: AWS gave no status when the listing was created (createdListingState), or the listing was created but neither persisted nor rolled back (keepUncanceledListing). Such a row holds the listing id and no attempt token. It was never selected, so it never became active, and a sale on it was never recorded. The store writes also refused it: RecordMarketplaceListingActive required active, and SettleMarketplaceListing required active or the same terminal state. The settlement integration test even listed this row shape as excluded, labeled "mid-claim".

Fix

Gap 1: pending rows that hold a listing id (internal/config/store_postgres_marketplace_settlement.go, internal/api/handler_marketplace_poller.go)

The store and the poller now handle a row that is pending, holds a listing_id and has listing_client_token IS NULL:

• ListMarketplaceListingsToPoll selects it.
• RecordMarketplaceListingActive moves it to active and stamps listed_at when AWS reports a CreateDate.
  • A zero listedAt now keeps the stored listed_at (COALESCE) instead of being written.
  • On an active row the statement behaves exactly as before and still never writes listing_state.
• SettleMarketplaceListing settles it to closed/cancelled like an active row.
• The poller calls RecordMarketplaceListingActive for a pending row even when AWS reports no CreateDate. Before, the poller skipped the write whenever CreateDate was missing. An active row without a CreateDate still gets no write.

A pending row with a token is still excluded from all three statements. The marketplace-list handler is mid-claim on that row, and the row's listing_id is the RI's previous, ended listing. If the poller wrote to it, it could overwrite the claim's pending lock with a terminal state.

Gap 3: deployment coverage (docs/DEPLOYMENT.md, schedule descriptions)

Making the poll run on every target means scheduling finalize_revocations on Fargate, GCP and Container Apps. That is infrastructure work, and #712 already tracks it. Per the issue's acceptance criterion ("or the docs say where it doesn't"), this PR documents the gap instead:

• New docs/DEPLOYMENT.md section, "RI Marketplace Listing Poll":
  • The poll runs inside finalize_revocations, which is scheduled hourly by default on AWS Lambda, CloudFormation and AKS (AKS only when deploy_kubernetes_resources is true).
  • Nothing schedules it on AWS Fargate, GCP Cloud Run, GKE or Azure Container Apps, so the poll does not run there.
  • Operators on those platforms can call POST /api/scheduled/finalize_revocations from their own scheduler.
• Toggle descriptions: the descriptions of enable_finalize_revocations_schedule (Terraform environments/aws, modules/compute/aws/lambda, modules/compute/azure/aks) and the CloudFormation EnableFinalizeRevocationsSchedule parameter now say that turning the schedule off also stops the marketplace poll.

The issue says the poll does not run on CloudFormation. That is out of date: the FinalizeRevocationsRule in cloudformation/stacks/CUDly/template.yaml now schedules it (default on). The coverage test's knownGaps (terraform/modules/compute/scheduled_task_coverage_test.go) agrees: aws-fargate, gcp-cloud-run, gcp-gke and azure-container-apps are the platforms without it.

Concurrency

Each write stays a single guarded UPDATE. Postgres re-checks the WHERE on the new row version after a lock wait, so a concurrent writer that commits first decides the outcome. Two interleavings matter:

• The cancel handler resolves the pending listing (ResolveMarketplaceListingAttempt / UpdatePurchaseHistoryListing):
  • If the cancel commits first, the poller's activate or settle no longer matches (the state is cancelled).
  • If the poll commits first, the cancel still applies, because ResolveMarketplaceListingAttempt also matches a row that holds the same listing_id. The cancel is always recorded.
• Cancel, then a relist claim, both commit while the poll waits. The row is pending again with the same listing_id, but it now holds a token, so the token guard drops the poller's write. Without the token guard the poller would turn the claim's pending lock into active or closed.

A claim can't start on a token-less pending row: ClaimMarketplaceListingSlot refuses it. So no claim races the pending → active move itself.

Verification

All of this is unit, mock and fixture evidence, plus real-Postgres integration tests (testcontainers). No AWS API was called: the poller is tested only with a fake listing finder.

Environment: Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true.

go build ./... && go vet ./... && go vet -tags integration ./...                       # clean
go test -count=1 ./internal/api/                                                        # ok
go test -count=1 ./internal/config/                                                     # ok
go test -count=1 ./internal/server/                                                     # ok
go test -count=1 ./internal/mocks/                                                      # ok
go test -count=1 ./terraform/modules/compute/                                           # ok (scheduled-task coverage / CFN tests)
golangci-lint v2.10.1 run --allow-parallel-runners [--build-tags integration] ./internal/api/ ./internal/config/   # 0 issues (both)
gocyclo -over 10 -ignore "_test\.go" internal/api/handler_marketplace_poller.go \
  internal/config/store_postgres_marketplace_settlement.go internal/config/interfaces.go   # nothing over 10
go test -tags integration -count=1 -timeout 20m \
  -run 'TestMarketplacePendingListingPoll_Postgres|TestMarketplaceListingSettlement_Postgres|TestMarketplaceListingRelist_Postgres' ./internal/config/   # ok
terraform fmt -check  (the three touched variables.tf files)                            # clean
terraform init -backend=false && terraform validate  on modules/compute/aws/lambda, modules/compute/azure/aks, environments/aws   # valid (temp TF_DATA_DIR, never plan/apply)
markdownlint-cli 0.47.0 -c .markdownlint.yaml docs/DEPLOYMENT.md                        # clean
CloudFormation template parsed with a tag-tolerant YAML loader; the edited Description reads back intact

New and changed tests

• TestPollMarketplaceListings_PendingRowBecomesActive (unit, internal/api): a pending row that AWS reports active is recorded through RecordMarketplaceListingActive, with and without a CreateDate.
• TestPollMarketplaceListings_ActiveRowWithoutCreateDateNeedsNoWrite and TestPollMarketplaceListings_PendingRowSettles (unit).
• TestMarketplacePendingListingPoll_Postgres (real Postgres, new file internal/config/store_postgres_marketplace_pending_poll_integration_test.go):
  • RecordedPendingBecomesActive: covers pending → active with and without a CreateDate, a zero date that must not clear listed_at, and a foreign listing id that must be ignored.
  • RecordedPendingSettles
  • MidClaimIsLeftAlone: covers a real ClaimMarketplaceListingSlot claim. Activate and settle must not apply, and the token must be kept.
  • CancelCommittingBeforeActivationWins: a deterministic lock-order race.
  • RelistClaimCommittingBeforePollIsLeftAlone/{activate,settle}: a deterministic lock-order race. The cancel and the claim commit while the poller's write waits on the row.
  • ParallelCancelAndPollKeepCancel: 10 rounds of ResolveMarketplaceListingAttempt, activate and settle started at once. The cancel is always recorded.
• TestMarketplaceListingSettlement_Postgres/PollSelection and /SettleIsGuarded:
  • The token-less pending row the old test listed as excluded ("mid-claim") is now expected to be polled.
  • The real mid-claim shape (a pending row with a token) is still excluded and still not settled.

Fail-before / pass-after

• Unit: with the poller restored from origin/main, TestPollMarketplaceListings_PendingRowBecomesActive/create_date_false fails because no write is made. It passes with the fix.
• Integration: with the store file restored from origin/main, these fail: PollSelection, RecordedPendingBecomesActive, RecordedPendingSettles, CancelCommittingBeforeActivationWins and RelistClaimCommittingBeforePollIsLeftAlone/{activate,settle}. The two race subtests fail because the old guard never matches the pending row, so the write never even waits on its lock. All of them pass with the fix.

Mutation probes, each applied alone to the fixed store and then reverted:

Mutation	Failing tests
Activate without listing_client_token IS NULL	MidClaimIsLeftAlone, RelistClaimCommittingBeforePollIsLeftAlone/activate
Settle without listing_client_token IS NULL	MidClaimIsLeftAlone, RelistClaimCommittingBeforePollIsLeftAlone/settle, SettleIsGuarded
Poll selection without the token guard	PollSelection
Zero listedAt written as NULL (no COALESCE)	RecordedPendingBecomesActive

Combined-tree verification at local queue commit 89fe5df6 = origin/main 3bfc2fe5 + fix/issue-1484, fix/issue-1498, fix/issue-1489 and fix/issue-1494 merged in that order. Passed: go build ./...; go vet with and without -tags integration; go test -race -short -count=1 ./...; golangci-lint v2.10.1 with and without the integration tag; gocyclo -over 10; go test -race -tags integration on internal/api, internal/config and internal/database/postgres/migrations; the git-secrets scan and scripts/test-git-secrets-allowlist.sh.

Out of scope

• Gap 2, a sale on a listing that has since been replaced. This needs a schema change, so I left it out (the brief for this branch excluded migrations).
  • purchase_history holds one listing's settlement per RI. A relist (UpdatePurchaseHistoryListing / ResolveMarketplaceListingAttempt with a new listing id) deliberately clears it, so the new listing can settle.
  • Keeping the earlier listing's proceeds once the row moves to the new listing needs per-listing storage, for example a marketplace_listing_settlements table keyed by listing_id.
  • Settling the old listing synchronously in the list or cancel path would not help: the settlement would still be overwritten when the new listing id is written.
  • The related "known window" from feat(marketplace): poll RI Marketplace listings and record proceeds and AWS fee (backend slice of #27) #1289 is also unchanged: a listing canceled through CUDly before its first poll never gets listed_at.
  • Owner decision: a per-listing settlement table (migration), or redefining the columns as a cumulative total per RI.
• The "Before exposing proceeds in the API or UI" items. One needs a real closed AWS listing (does AWS keep a price tier marked active after a listing ends?). The other is an owner decision (estimated proceeds versus actual disbursement data). Neither was attempted, and no AWS call was made.
• Scheduling finalize_revocations on Fargate, GCP and Azure Container Apps. Tracked by fix(iac): several scheduled tasks are not scheduled; delayed purchases never fire on GCP/Azure #712. Only the docs changed here.

Frontend follow-up for the UI owner

None needed for this change. Nothing under frontend/ was touched. A listing recorded as pending now becomes active (or closed/cancelled) on the next poll, so the history view's existing state display picks it up.

Labels

A maintainer please mirror #1494's triage labels: priority/p2, type/bug. The issue carries no severity/*, urgency/*, impact/*, effort/* or triaged labels.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 11 billable files and costs up to $2.75.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 3bfc2fe
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: e5cc3748-9119-4542-8432-fc82d92334a3

Commits

Reviewing files that changed from the base of the PR and between 3bfc2fe and d1dd5af.

Files selected for processing: 11

• cloudformation/stacks/CUDly/template.yaml
• docs/DEPLOYMENT.md
• internal/api/handler_marketplace_poller.go
• internal/api/handler_marketplace_poller_test.go
• internal/config/interfaces.go
• internal/config/store_postgres_marketplace_pending_poll_integration_test.go
• internal/config/store_postgres_marketplace_settlement.go
• internal/config/store_postgres_marketplace_settlement_integration_test.go
• terraform/environments/aws/variables.tf
• terraform/modules/compute/aws/lambda/variables.tf
• terraform/modules/compute/azure/aks/variables.tf

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

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

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE d1dd5af54cb5b5d57ef551b4a15c9c56bf8161e2

Blocking:

• none

Non-blocking:

• internal/api/handler_marketplace_poller.go:120 / store_postgres_marketplace_settlement.go:36: a token-less pending row that has already ended on AWS when it is first polled, sold units at an unknown price, and comes back without a CreateDate is settled with listed_at NULL and no proceeds. The terminal branch of ListMarketplaceListingsToPoll needs listed_at IS NOT NULL, so that row is never polled again and its proceeds stay unrecorded. Active rows already had this hole (it is the feat(marketplace): poll RI Marketplace listings and record proceeds and AWS fee (backend slice of #27) #1289 "known window"), and AWS normally returns CreateDate, so this is an edge case. It could be closed later by letting the terminal branch also match listed_at IS NULL rows the poller settled.
• internal/api/handler_marketplace.go:326 (unchanged): keepUncanceledListing still tells the user a pending listing "needs reconciliation". The poller now reconciles it automatically, so the wording could mention that.
• ParallelCancelAndPollKeepCancel checks that a cancel overwrites a poller-written closed. That ordering is pre-existing behavior of ResolveMarketplaceListingAttempt (matched by listing_id), not something this PR introduced. Noted only so nobody reads the test as a new decision.

Attacked:

• Traced every writer of listing_state = 'pending' to check that "pending + listing_id + NULL token" only means a real recorded listing, never the previous ended listing of an in-flight claim:
  • UpdatePurchaseHistoryListing via createdListingState, reconcileAttempt and keepUncanceledListing always writes the new listing id and clears the token.
  • ReleaseMarketplaceListingClaim never restores pending (releaseMarketplaceClaim maps a prior pending to "").
  • ClaimMarketplaceListingSlot always leaves a token.
  • So the token guard separates the two shapes correctly.
• Checked the interplay with the cancel handler (recoverPendingListing → ResolveMarketplaceListingAttempt, which also matches by listing_id, so the cancel lands after a poll), and with a relist after the poller settles a pending row (claim now allowed, as intended).
• Verified the docs and description claims against scheduled_task_coverage_test.go knownGaps, the CFN FinalizeRevocationsRule and the TF/AKS defaults, and that /api/scheduled/finalize_revocations runs handleFinalizeSweep.
• Ran: go build ./...; go vet with and without -tags integration on api/config/server; go test -race -short -count=1 on internal/api, internal/config, internal/server and terraform/modules/compute (all ok); go test -race -tags integration -run Marketplace ./internal/config/ (ok).
• Fail-before: restored the poller and settlement store from origin/main.
  • TestPollMarketplaceListings_PendingRowBecomesActive fails.
  • So do RecordedPendingBecomesActive, RecordedPendingSettles, CancelCommittingBeforeActivationWins, RelistClaimCommittingBeforePollIsLeftAlone/{activate,settle} and PollSelection.
  • All pass with the fix restored.

Body-stale: none. The PR correctly says Refs (not Closes), because gap 2 and the "before exposing proceeds" items are left open.

_{Reviewer: rF1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at d1dd5af54cb5b5d57ef551b4a15c9c56bf8161e2 merged onto main (30f5602b), together with #1505, #1508 in that order (combined tree 7b4062ee, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on d1dd5af5, green CI, and the combined-tree verification above. Squash-merged at exactly d1dd5af54cb5b5d57ef551b4a15c9c56bf8161e2.