[IAmJSD]

Problem (gap 2 of the issue)

purchase_history describes only an RI's latest marketplace listing. A relist, or the claim that starts one, moves the row on to the new listing on purpose (#1289): listing_id changes and the previous listing's listed_at/proceeds/fee are cleared. So if listing A sold partially and was canceled, and the RI was relisted as B before a poll saw A's sale, nothing named A any more. The poller never read A again and A's proceeds were lost. #1508 fixed gap 1 and documented gap 3. This PR fixes gap 2 as the acting owner decided (D24): each listing's settlement goes in its own row.

Fix

Migration 000153 (internal/database/postgres/migrations/000153_marketplace_listing_settlements.{up,down}.sql)

• New table marketplace_listing_settlements, primary key listing_id (AWS listing ids are unique, and each belongs to one RI). It has purchase_history_id (FK to purchase_history.id, ON DELETE CASCADE), purchase_id, status, listed_at, proceeds_received, fee_paid (NUMERIC(12,2)), currency (default USD), settled_at, created_at and updated_at. A CHECK makes proceeds, fee and settled_at all set or all NULL. There are indexes on purchase_id and purchase_history_id.
• Backfill: one row for each listing that purchase_history currently has a settlement for (listing_proceeds_received IS NOT NULL). Legacy duplicate rows of one RI hold the same listing, so the backfill uses the earliest row.
• Trigger purchase_history_keep_superseded_listing (AFTER UPDATE OF listing_id, listing_state, listing_client_token) copies a listing into the table, as purchase_history last recorded it, at the moment the row stops describing it. That happens when listing_id changes, or when a list claim makes the row pending with a token. The claimed row keeps the old listing_id but its state becomes pending, so the claim is the last moment the old listing's real state is known.
  • A row that is already pending with a token is never copied: its state belongs to the in-flight attempt, not to the listing id it holds.
  • A row whose attempt was released with its token kept (an ambiguous AWS outcome) is back in the listing's own state, so the retry's claim does copy it.
  • ON CONFLICT DO NOTHING keeps any row the poller already wrote.
• The header comment includes a read-only detection query (settled listings on purchase_history with no table row) and says what the down migration restores.
• Down: drops the trigger, the function and the table, which restores the 000152 schema. purchase_history's columns were never touched, so they still hold each RI's latest listing settlement. Settlements of superseded listings exist only in the table and are lost on rollback.

Store (internal/config/store_postgres_marketplace_settlement.go)

• ListMarketplaceListingsToPoll reads the current listing from purchase_history (unchanged rules) UNION listings kept in the table that are still unsettled. The same rules apply to both: active, pending, or closed/canceled with listed_at set. Both leave out a listing that a purchase_history row holds as a claim in flight (pending with a token), as fix(marketplace): poll listings recorded as pending and document where the poll runs #1508 does. Account, region and cloud account come from the referenced purchase_history row. Results are deduplicated per (purchase_id, listing_id), and the purchase_history copy wins when both sources have a listing.
• SettleMarketplaceListing now runs one transaction:
  1. The existing guarded purchase_history UPDATE, unchanged. It applies only while the row still holds the listing.
  2. SELECT … FOR SHARE on the purchase_history rows that hold the listing. If any of them is a claim in flight, nothing more is written. This is fix(marketplace): poll listings recorded as pending and document where the poll runs #1508's mid-claim rule, now applied to the per-listing row too. The share lock makes a claim that starts meanwhile wait for this transaction, so no settlement is ever written while a row is mid-claim.
  3. If step 1 applied, an upsert into the listing's own row (INSERT … ON CONFLICT (listing_id) DO UPDATE … WHERE proceeds_received IS NULL AND status IN (active, pending, same terminal state) AND same purchase_id). Otherwise, the same guarded UPDATE runs against the row the relist kept.
     It returns true if either write applied. Locks are taken on purchase_history first and then on the table, the same order a relist and its trigger use, so a poll and a relist cannot deadlock.
• Claims in flight lose nothing. A listing whose sale a poll skipped mid-claim is still unsettled in the kept row. Once the claim resolves to the new listing, it is polled from the table. If the claim is released, it is polled from purchase_history again. Either way it is settled once.
• New read path: GetMarketplaceListingProceedsTotals(ctx, purchaseIDs) returns, for each RI and currency, the sum of proceeds and fees over its settled listings, plus counts of settled and unsettled listings. It reads the table only. It is on StoreInterface and has a mock.

What purchase_history's settlement columns mean now: still the latest listing's settlement, not a total. The poller's own guards depend on that meaning (listing_proceeds_received IS NULL means "this listing is not settled yet", and RecordMarketplaceListingActive and the relist paths clear the columns for a new listing). Turning the columns into a running total would break those guards. Anyone who added the total to the per-listing rows would also count a sale twice. Per-RI totals therefore come from the table only.

Where proceeds totals are read today: nowhere. No API handler, report or frontend code reads listing_proceeds_received/listing_fee_paid (the frontend's history.ts computes its own pre-listing estimate). So the new read path has no caller yet. It is the method the API should use when proceeds are exposed (that is gated on D23, below).

Money path: no double counting. Each listing has exactly one row (the primary key). Each write fills a row only while its amounts are NULL, and the row lock serializes concurrent pollers. Totals sum the table only.

Closes #1494

Verification

Commands, run from the worktree with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true and Go 1.26.9:

• go build ./..., go vet ./... and go vet -tags integration ./... all pass.
• go test -count=1 passes for ./internal/config/, ./internal/mocks/, ./internal/api/ and ./internal/database/postgres/migrations/ (run one package at a time).
• golangci-lint run --allow-parallel-runners --build-tags integration ./internal/config/ ./internal/mocks/ ./internal/api/ ./internal/database/postgres/migrations/ reports 0 issues.
• gocyclo -over 10 -ignore "_test\.go" on the changed non-test files reports nothing.
• Real Postgres (testcontainers, postgres:16-alpine), my new tests only:
  • Whole package: go test -tags integration -count=1 -timeout 30m ./internal/config/ passes (96s, at 568c6d6 with origin/main 6e2033c merged). This includes fix(marketplace): poll listings recorded as pending and document where the poll runs #1508's TestMarketplacePendingListingPoll_Postgres/MidClaimIsLeftAlone, which failed on the first commit of this branch in the combined-tree run.
  • go test -tags integration -count=1 -timeout 20m -run 'TestMarketplaceListingSettlements_Postgres' ./internal/config/ passes, and also passes with -race. Subtests:
    • RelistAfterPartialSaleKeepsBothListings: A is canceled after the poller saw it, the RI is relisted as B, A is still returned by the poll, both settle, the RI total is A+B (264/36), and purchase_history holds B's settlement.
    • RepollIsIdempotent: three more polls of each listing with different amounts change nothing.
    • UnpricedSupersededListingIsPolledUntilPriced
    • ReleasedClaimCountsListingOnce: while the claim is in flight, A is not polled and a settle writes nothing. A failed relist then releases the claim, so A is both on the row and in the table. It is polled once and counted once.
    • SettleWaitingOnClaimLeavesListingForLater: deterministic held-lock race. The poll queues behind a committing claim and writes nothing (fix(marketplace): poll listings recorded as pending and document where the poll runs #1508). Once the claim resolves to B, A is polled from the kept row and recorded once.
    • ResumedClaimKeepsListing: an attempt released with its token kept (written with triggers off, as if from before 000153). The retry's claim keeps A, so the relist does not lose it.
    • ConcurrentPollsSettleSupersededOnce: 8 pollers × 5 rounds, exactly one applies.
    • ConcurrentRelistAndPollKeepProceedsOnce: 20 rounds of relist + 2 polls. Even rounds start all three at once; odd rounds start the polls after the claim commits, so both ways the listing's row can be created are exercised (the claim created it in 10–12 of 20 rounds). In every round:
      • at most one poll records the sale;
      • if neither did (the polls ran mid-claim, which happened in about 2 of 20 rounds), the listing is still due and the next poll records it;
      • the final total is 88/12, and no deadlock happens.
    • TotalsOfUnknownPurchases
  • go test -tags integration -count=1 -timeout 20m -run 'TestMigration153_MarketplaceListingSettlements' ./internal/database/postgres/migrations/ passes. It migrates to previousMigrationVersion(t, 153), seeds settled, legacy-duplicate, unsettled and unlisted rows, then migrates up. It checks the backfill (2 rows, values, earliest duplicate), that the trigger keeps a relisted listing with its recorded state and listed_at, and that the amounts CHECK holds. It then migrates down (table, trigger and function gone, purchase_history values kept, a relist still works) and re-applies.
• Mutation probes (real Postgres):
  • Removing the mid-claim check from the settle path makes MidClaimIsLeftAlone, ReleasedClaimCountsListingOnce and SettleWaitingOnClaimLeavesListingForLater fail.
  • Restoring the first commit's trigger condition makes ResumedClaimKeepsListing fail.
• Fail-before: origin/main (3ec21a0) store code, with only the new migration, the new test file and the totals read path copied in. 8 of the 9 subtests fail: every subtest above except TotalsOfUnknownPurchases (count confirmed by both reviewers at 568c6d6). For example, "ril-mls-b" map … does not contain "ril-mls-a" (the superseded listing is not polled), and round 1: exactly one poll records the sale. The pass-after results are listed above.

This is DB-level integration evidence on real Postgres. No AWS call is made; the poller's AWS read is not part of these tests.

Combined-tree verification. Tested on a local, never-pushed tree: queue/platform @ 19e51df6 = origin/main 6e2033cd plus:

Branch	Tested head
fix/issue-1494-listing-settlements	568c6d6
fix/issue-1529-ambiguous-return	8ecf506
fix/issue-1434-lambda-budget	8b6eebe
fix/issue-1432-retire-cfn-stack	9cb7de8
fix/issue-712-fire-default-aws-schedule	8bd7095

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers.

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests (-tags integration -count=1, one package at a time): internal/api, internal/config, internal/database/postgres/migrations, internal/purchase and internal/server pass.
• Secrets: git-secrets on the changed files found nothing, and a planted fake key was flagged; scripts/test-git-secrets-allowlist.sh passes.
• Merge order: fix(cfn): CloudFormation CUDly stack can't start the current server (no DB_HOST; still provisions DynamoDB) #1432 (retire the CloudFormation stack) must merge before the fire-default branch. On its own, that branch fails two tests in the CloudFormation stack's test file, which fix(cfn): CloudFormation CUDly stack can't start the current server (no DB_HOST; still provisions DynamoDB) #1432 deletes.

Deliberately out of scope / notes for review

• The "Before exposing proceeds" items of fix(marketplace): close RI Marketplace poller gaps (pending rows with a listing id, superseded listings, deployment coverage) #1494 are not done here. (1) Checking one real closed AWS listing to confirm the price tier stays active needs a live seller account. (2) The decision to label proceeds as estimates (D23) only matters once proceeds are exposed. Nothing exposes proceeds yet, so they do not block this backend change. They need to be handled when an API or UI change exposes proceeds.
• The "known window" from feat(marketplace): poll RI Marketplace listings and record proceeds and AWS fee (backend slice of #27) #1289 is unchanged on purpose. A listing canceled before the poller ever saw it (listed_at NULL) is still not polled, either on purchase_history or in the table. The table keeps it (unsettled), so a later change could poll it.
• Rows created before this migration cannot be recovered. A listing that was already superseded and lost before 000153 left no record. A claim that is already in flight when 000153 runs also lost its previous listing's state when it started. That listing is still lost when the claim resolves, as it is on main today.
• A claim that stays stuck in flight keeps its previous listing unpolled until the cancel endpoint's attempt recovery (fix(marketplace): ambiguous AWS errors and crashes between create and persist can leave a duplicate or a stuck pending row #525) resolves or releases it. This is the same as fix(marketplace): poll listings recorded as pending and document where the poll runs #1508's rule for the purchase_history row.
• If two different purchases ever recorded the same listing id (AWS never does this; some older tests do it on purpose), the table keeps only the first purchase's row. The second purchase's settlement then goes only on its purchase_history row, so a sale is never counted twice.
• ON DELETE CASCADE: deleting the purchase_history row a listing references deletes that listing's row. No code path deletes purchase_history rows today.
• Migration number: the brief fixed 000153, but open draft PR fix(db): contract the canceled rename: normalize, narrow the status CHECKs, stop using cancelled_by #1253 (000153_contract_canceled_spelling) also uses 000153. Whichever PR merges second has to renumber.

Frontend follow-up for the UI owner

No frontend change in this PR. Once proceeds are exposed in the API (after D23 and the real-listing check above), the History view's marketplace column should show each RI's total across listings, from GetMarketplaceListingProceedsTotals, labeled as an estimate. A per-listing breakdown could be shown as well. It should not read purchase_history's latest-listing columns as the RI's total.

Maintainer: please mirror the issue's triage labels onto this PR: priority/p2, type/bug. The issue has no triaged label, so do not add that one.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{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 568c6d619da0f81e1d13c9fb5bf2e15f0d9891bd

Blocking:

• none

Non-blocking:

• internal/config/store_postgres_marketplace_settlement.go (GetMarketplaceListingProceedsTotals): totals read only marketplace_listing_settlements. A settlement written to purchase_history alone after 000153 runs, for example by a pod still on the old image during a rolling deploy, has no table row. The poller won't fix that, because listing_proceeds_received is set. The row is only created when the RI is next claimed for a relist, and the trigger then copies it. Until that happens the RI's total is too low. Nothing calls the method yet, so this is harmless today. Before the D23 API change starts reading totals, re-run the backfill INSERT (it is idempotent through ON CONFLICT DO NOTHING), or run the header's detection query once and confirm it returns nothing.
• Same function: SUM(NUMERIC(12,2))::float8 turns money into float64. When this is exposed, return cents or a decimal string.
• The trigger also keeps listings in the feat(marketplace): poll RI Marketplace listings and record proceeds and AWS fee (backend slice of #27) #1289 known window (canceled, listed_at NULL). Nothing polls them, so they stay in UnsettledListings forever. A future UI should not treat UnsettledListings > 0 as "money still coming".
• Legacy duplicate rows (pre-000108) where the earliest row is mid-claim: step 1 can settle a non-claimed duplicate and return true while the table write is skipped. The table row is filled by a later poll once the claim resolves, through the superseded path, and is still counted once. This is fine, but the doc comment says "neither is written", which is not quite true for duplicates.

Attacked: Read the migration, trigger WHEN clause, poll UNION, and the 3-step settle tx against #1494's acceptance criteria. Gap 1 and gap 3 were already handled by merged #1508, so Closes is correct. Checked lock order (purchase_history X/SHARE, then the table row) for poll vs relist, poll vs poll, and claim-waits-on-share. Found no lock cycle. EvalPlanQual drops the settle UPDATE when a concurrent relist moved listing_id. Checked that FK and column types match purchase_history (UUID, VARCHAR(255), NUMERIC(12,2)), that the down migration drops cleanly, and that 000153 follows main's 000152. Ran: go build ./..., go vet ./..., go vet -tags integration ./... (all OK). go test -race -short -count=1 on internal/{config,mocks,api,database/postgres/migrations}: ok. go test -race -tags integration -run TestMarketplace ./internal/config/: ok. -run TestMigration153 ./internal/database/postgres/migrations/: ok. Fail-before (real Postgres): with origin/main's store_postgres_marketplace_settlement.go plus only the new totals func, 8 of 9 TestMarketplaceListingSettlements_Postgres subtests fail (e.g. does not contain "ril-mls-a"). With the PR's store code and only the CREATE TRIGGER removed from 000153, the same 8 fail.

Body-stale:

• "7 of the 8 subtests fail": the test has 9 subtests, and 8 of them fail before the fix.
• "Whichever PR merges second has to renumber": common.txt now assigns 000153 to this PR, and fix(db): contract the canceled rename: normalize, narrow the status CHECKs, stop using cancelled_by #1253 renumbers.

_{Reviewer: rN1. 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]

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

Blocking:

• none

Non-blocking:

• internal/config/store_postgres_marketplace_settlement.go:319-330: GetMarketplaceListingProceedsTotals filters by purchase_id only, with no account_id. It has no caller today. When the API exposes it (after D23), the handler must pass only purchase IDs already scoped to the caller's account. Otherwise any reservation id a user can guess returns that RI's totals. Consider taking an account id now so the scoping can't be forgotten.

• internal/config/store_postgres_marketplace_settlement.go:325-326: money is summed as NUMERIC and then cast to float8. That matches the existing *float64 settlement fields, but an API that shows totals should format to cents and not compare floats.

• Superseded listings that never become priced (the issue's open "price tier stays active" question) are now re-polled every hour per listing, not per RI. Before, an RI had at most one due listing. Now each relist can leave another one behind, so the number of hourly AWS calls can grow. This is bounded by how often RIs are relisted, so it's minor, but it makes the real-listing check more important before exposure.

• A superseded listing copied while active, or pending without a token, stays active/pending in the table. RecordMarketplaceListingActive only writes to purchase_history. None of the current call paths can supersede such a row: the claim refuses active rows and pending rows without a token, and every listing-id change happens on a claimed row. If one ever did, it would only cost hourly polls until AWS ends the listing, and the settle guard would still accept it.

• 000153 up.sql:76: ON DELETE CASCADE follows a legacy duplicate purchase_history row (the trigger records the first row it updated). A future duplicate-cleanup migration that deletes duplicates would silently drop superseded listings' settlements with them. Worth a comment, or RESTRICT, if dedupe is ever planned.

• Closing fix(marketplace): close RI Marketplace poller gaps (pending rows with a listing id, superseded listings, deployment coverage) #1494 also closes its "Before exposing proceeds" items (the real closed-listing check and D23). Those aren't acceptance criteria, so Closes is fair, but they should go to a follow-up issue so they aren't lost.

• (From the earlier rN1 review; I missed it.) If an old-image pod settles a current listing after 000153 runs, for example during a rolling deploy, it writes only purchase_history. That listing gets no table row until the RI is next claimed, so totals stay low until then. Before the D23 API starts reading totals, run the header's detection query (or re-run the idempotent backfill).
  Attacked:

• I checked the trigger's WHEN against every writer of listing_id/listing_state/listing_client_token: the claim (incl. resumed and released-keep-token), UpdatePurchaseHistoryListing (all 6 call sites, per-row on legacy duplicates), release, the cancel endpoint, and both settle and activate. I also traced the lock order through the claim/settle/relist interleavings (settle waits on a claim's row lock and then sees mid-claim; a claim waits on a settle, and its trigger insert hits ON CONFLICT DO NOTHING). I found no double count and no deadlock.

• Checks: go build ./..., go vet ./..., go vet -tags integration ./...: OK. go test -race -short -count=1 on config, api, mocks and migrations: OK. go test -race -tags integration -count=1 on the whole of ./internal/config/ (138s) and ./internal/database/postgres/migrations/ (233s): OK. golangci-lint v2.10.1 with and without --build-tags integration: 0 issues. gocyclo -over 10 on the changed non-test files: silent.

• Fail-before (real Postgres): I swapped the store file back to origin/main's, appended only the totals func, and kept the migration. 8 of 9 TestMarketplaceListingSettlements_Postgres subtests fail; only TotalsOfUnknownPurchases passes.

• Mutation probes: dropping the trigger's claim clause fails 8/9 subtests. Dropping proceeds_received IS NULL from settleSupersededListingSQL fails RepollIsIdempotent, ReleasedClaimCountsListingOnce, ConcurrentPollsSettleSupersededOnce and ConcurrentRelistAndPollKeepProceedsOnce. So the tests do pin the money guards. I restored everything afterwards.

Body-stale:

• "7 of the 8 subtests fail": there are now 9 subtests (ResumedClaimKeepsListing was added), and 8 of them fail before the fix. The substance is unchanged.

_{Reviewer: rN2. 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 568c6d619da0f81e1d13c9fb5bf2e15f0d9891bd merged onto main (8761aa4b), together with #1581 in that order (combined tree 5ed232d9, 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 2 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: 2 approving independent review(s) on 568c6d61, green CI, and the combined-tree verification above. Squash-merged at exactly 568c6d619da0f81e1d13c9fb5bf2e15f0d9891bd.