Skip to content

feat(marketplace): sell/cancel Standard RIs on AWS Marketplace (closes #292)#808

Open
cristim wants to merge 12 commits into
feat/multicloud-web-frontendfrom
fix/292-wave4
Open

feat(marketplace): sell/cancel Standard RIs on AWS Marketplace (closes #292)#808
cristim wants to merge 12 commits into
feat/multicloud-web-frontendfrom
fix/292-wave4

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented May 28, 2026

Summary

  • Migration 000060: adds offering_class, listing_id, listing_state columns to purchase_history
  • Backend: POST /api/purchases/{id}/marketplace-list and POST /api/purchases/{id}/marketplace-cancel routes with sell-any/sell-own RBAC (mirroring the cancel/retry/approve family); sell-own added to DefaultUserPermissions; default price schedule is 95% of residual value (upfront + monthly * remaining months); EC2 API: CreateReservedInstancesListing, DescribeReservedInstancesListings, CancelReservedInstancesListing
  • Frontend: "Sell on Marketplace" button on completed Standard RI rows; "Cancel listing" button when listing_state == "active"; confirm-dialog + API call + toast + reload click handlers; all API types exported from api/index.ts

Test plan

  • Run go test ./... - all 4966 tests pass across 38 packages
  • Run npm test in frontend/ - all 2142 frontend tests pass
  • Frontend build: npm run build - compiles with no TS errors
  • Smoke-test: create a Standard RI execution row with offering_class=standard and verify "Sell on Marketplace" button appears; click it and confirm the API call is dispatched
  • Verify the "Cancel listing" button appears when listing_state=active and cancels correctly
  • Verify Convertible RI rows and non-completed rows do NOT show the Sell button
  • Verify sell-own users can only sell RIs in their allowed accounts; sell-any users can sell any

Scope / descope

This PR ships listing creation and cancellation for Standard RIs. Two parts of #292 are intentionally descoped to keep it landable and are tracked in follow-up #966:

  • Status poller (active -> closed/sold transition, realized proceeds, AWS fee actually charged).
  • Seller-account precondition UI (registered seller / US bank account on file) before listing.

Because the poller is not implemented, migration 000060 was reduced to only the columns the implemented flow reads/writes (offering_class, listing_id, listing_state). The settlement/poller columns (listed_at, listing_price_schedule, listing_proceeds_received, listing_fee_paid) were removed so the schema never carries columns nothing populates; they will land with the poller in #966.

Closes #292 partially; remainder tracked in #966.

Summary by CodeRabbit

  • New Features
    • AWS Reserved Instance Marketplace support: eligible Standard RIs can be listed for sale with customizable price tiers and visible AWS fees/net proceeds.
    • "Sell on Marketplace" button shown on eligible RI rows; confirms pricing before listing.
    • "Cancel Listing" button to remove active marketplace listings.
    • New marketplace selling permission added for non-admin users.

@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/m Days type/feat New capability labels May 28, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e207a2b-8f26-4fa1-a5e1-e19168abccc5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Implements AWS RI Marketplace listing and cancellation end-to-end: new auth actions, DB schema and store methods, EC2 client ops, API handlers/routes, frontend API wrappers and History UI buttons with confirm flows, and test/mock updates.

Changes

AWS RI Marketplace Listing Implementation

Layer / File(s) Summary
Authorization Model and Permissions
internal/auth/types.go, internal/auth/types_test.go, internal/auth/service_group_test.go, frontend/src/permissions.generated.ts
Adds ActionSellOwn and ActionSellAny constants; grants sell-own:purchases to default users; updates tests and generated frontend permissions.
Data Models and Type Definitions
internal/config/types.go, frontend/src/types.ts
Adds marketplace metadata fields (OfferingClass, ListingID, ListingState) to backend PurchaseHistoryRecord and frontend HistoryPurchase types; documents JSON/DynamoDB tags and usage.
Database Schema and Store Interface
internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.up.sql, ...down.sql, internal/config/interfaces.go
Migration 000060 adds nullable marketplace columns to purchase_history (including offering_class, listing_id, listing_state, listed_at, listing_price_schedule, listing_proceeds_received, listing_fee_paid); StoreInterface gains GetPurchaseHistoryByPurchaseID and UpdatePurchaseHistoryListing.
PostgreSQL Implementation and Test Fixtures
internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go
Implements UpdatePurchaseHistoryListing and GetPurchaseHistoryByPurchaseID; extends SavePurchaseHistory insert/args for offering_class; updates select/scan paths to include marketplace columns and refactors row scanning helper; updates pgxmock fixtures and INSERT bind counts.
AWS EC2 Marketplace API Client
providers/aws/services/ec2/client.go, providers/aws/services/ec2/client_test.go
Extends EC2API with Create/Describe/Cancel listing calls; adds MarketplaceListingRequest, MarketplacePriceTier, MarketplaceListingResult; implements CreateMarketplaceListing, DescribeMarketplaceListing, CancelMarketplaceListing and corresponding mock methods.
Handler Injection and EC2 Client Factory
internal/api/handler.go
Adds optional marketplaceEC2Factory DI field to Handler for test injection with production fallback.
Marketplace Listing/Cancellation Handlers
internal/api/handler_marketplace.go
New handler module with MarketplacePriceTier, request/response types, validateMarketplaceListRequest, marketplaceList, marketplaceCancel, RBAC helpers, remaining-term calculation, AWS error mapping, price-schedule resolution, EC2 calls, and DB persistence with rollback on write failure when applicable.
API Route Registration
internal/api/router.go
Registers POST /api/purchases/{id}/marketplace-list and POST /api/purchases/{id}/marketplace-cancel with AuthUser; adds Router wrapper methods.
Frontend API Wrappers
frontend/src/api/purchases.ts, frontend/src/api/index.ts
Adds createMarketplaceListing and cancelMarketplaceListing async wrappers and TypeScript types; barrel export updated to include functions and types; createMarketplaceListing conditionally includes price_schedule when provided.
History Table Marketplace UI and Event Handlers
frontend/src/history.ts
Adds canSellOnMarketplace eligibility check (completed, standard offering, no active listing, authenticated, ≥1 month remaining); includes marketplace buttons in same-row disabling; renders Sell/Cancel buttons in action cell; wires click handlers to show confirm modal, call API, show toasts, disable same-row actions, and reload history.
Mock Implementations and Test Support
internal/analytics/collector_test.go, internal/api/mocks_test.go, internal/mocks/stores.go, internal/purchase/mocks_test.go, internal/scheduler/scheduler_test.go, internal/server/test_helpers_test.go
Adds GetPurchaseHistoryByPurchaseID and UpdatePurchaseHistoryListing mock/stub methods across tests and helpers using testify m.Called(...) with nil-safe handling.

Sequence Diagram(s)

sequenceDiagram
  participant User as User (Browser)
  participant History as History UI
  participant FrontendAPI as Frontend API
  participant Router as API Router
  participant Handler as Marketplace Handler
  participant EC2 as AWS EC2
  participant DB as PostgreSQL

  User->>History: Click "Sell on Marketplace"
  History->>History: Show pricing confirm modal
  User->>History: Confirm price & AWS fee
  History->>FrontendAPI: createMarketplaceListing(purchaseId, schedule)
  FrontendAPI->>Router: POST /purchases/{id}/marketplace-list
  Router->>Handler: validateMarketplaceListRequest + marketplaceList
  Handler->>Handler: Check session, purchase, offering_class, RBAC
  Handler->>Handler: Compute remaining months & price schedule
  Handler->>EC2: CreateReservedInstancesListing
  EC2-->>Handler: listing_id, listing_state
  Handler->>DB: UpdatePurchaseHistoryListing(listing_id, listing_state)
  DB-->>Handler: success
  Handler-->>Router: MarketplaceListResponse
  Router-->>FrontendAPI: listing_id, listing_state, price_schedule
  FrontendAPI-->>History: MarketplaceListResult
  History->>History: Show success toast
  History->>History: Reload history & show "Cancel listing" button
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

severity/medium

Poem

🐰 I hopped through code with nimble feet,
To help Standard RIs find buyers neat,
Buttons, handlers, DB and fee,
A marketplace dance from A to Z!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately summarizes the main feature: marketplace selling and cancellation for Standard RIs, matching the primary objective and linked issue #292.
Linked Issues check ✅ Passed PR implements all core marketplace requirements: Standard RI listing [#292], cancel listing [#292], RBAC sell-own/sell-any [#292], price schedule with default [#292], AWS Marketplace precondition validation [#292], EC2 API integration [#292], database persistence [#292], and UI affordance [#292].
Out of Scope Changes check ✅ Passed All changes directly support marketplace listing/cancellation for Standard RIs. No unrelated refactoring or scope creep detected beyond the core feature.
Docstring Coverage ✅ Passed Docstring coverage is 82.61% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/292-wave4

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 28, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/history.ts (1)

610-624: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't let retry-lineage markup suppress marketplace actions.

Line 610 returns as soon as lineage exists, so a completed Standard RI with retry_attempt_n > 0 never reaches Lines 617-623. That hides Sell/Cancel for successful retry descendants, even though they're still eligible completed purchases.

Suggested direction
-  if (lineage.length > 0) {
-    return lineage.join(' ');
-  }
-
-  // Completed Standard RI rows: offer Sell on Marketplace (issue `#292`).
-  // The button is placed here (after lineage links) so it does not
-  // interfere with the Retry / Approve / Cancel affordances above.
-  if (p.purchase_id) {
-    if (canCancelMarketplaceListing(p)) {
-      return `<button type="button" class="btn-link history-marketplace-cancel-btn" data-marketplace-cancel-id="${escapeHtml(p.purchase_id)}">Cancel listing ${escapeHtml(p.listing_id || '')}</button>`;
-    }
-    if (canSellOnMarketplace(p)) {
-      return `<button type="button" class="btn-link history-marketplace-sell-btn" data-marketplace-sell-id="${escapeHtml(p.purchase_id)}">Sell on Marketplace</button>`;
-    }
-  }
+  const trailingActions: string[] = [];
+  if (p.purchase_id) {
+    if (canCancelMarketplaceListing(p)) {
+      trailingActions.push(
+        `<button type="button" class="btn-link history-marketplace-cancel-btn" data-marketplace-cancel-id="${escapeHtml(p.purchase_id)}">Cancel listing ${escapeHtml(p.listing_id || '')}</button>`,
+      );
+    } else if (canSellOnMarketplace(p)) {
+      trailingActions.push(
+        `<button type="button" class="btn-link history-marketplace-sell-btn" data-marketplace-sell-id="${escapeHtml(p.purchase_id)}">Sell on Marketplace</button>`,
+      );
+    }
+  }
+
+  if (lineage.length > 0 || trailingActions.length > 0) {
+    return [...lineage, ...trailingActions].join(' ');
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/history.ts` around lines 610 - 624, The early return when
lineage.length > 0 prevents the marketplace buttons from being rendered for
completed retry descendants; update the logic in the function handling the
history row so that when lineage exists you append (or merge) the marketplace
button markup instead of returning immediately — locate the lineage handling and
the marketplace block using lineage, p.purchase_id, canCancelMarketplaceListing,
canSellOnMarketplace and escapeHtml; either move the marketplace block above the
return or build a combined string (lineage.join(' ') + marketplaceMarkup) so
Sell on Marketplace / Cancel listing buttons are included for eligible
purchases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/history.ts`:
- Around line 914-943: The flow skips the required pricing modal: after the
initial confirmDialog in the '.history-marketplace-sell-btn' click handler, open
the RI pricing/pricing-schedule modal (e.g., call a new or existing
showMarketplacePricingModal / showPricingModal function) to display the RI
summary, default per-month price schedule and 12% fee breakdown and allow the
user to adjust/select schedule types (use the schedule types exposed by
frontend/src/api/purchases.ts via api.getPricingSchedules or similar). Only if
the user confirms the pricing modal proceed to call
api.createMarketplaceListing(id); preserve the existing UI state handling
(disable/enable sameRowActions(btn) and call loadHistory() on success) and
handle errors exactly as existing catch blocks do.
- Around line 474-496: canSellOnMarketplace currently omits the required
remaining-term check; update the function (canSellOnMarketplace(p:
HistoryPurchase)) to return false unless the purchase has at least one month
remaining before maturity by inspecting the appropriate remaining-term field on
p (e.g., p.remaining_term, p.remaining_months, or p.termRemaining depending on
the model) and parsing/coercing string values to numbers if needed; add this
gate alongside the existing status/offering_class/listing_state checks so
matured Standard RIs (remaining < 1) do not show the Sell button.

In `@internal/api/handler_marketplace.go`:
- Line 147: The code currently maps every AWS marketplace error to HTTP 502 via
NewClientError("AWS marketplace listing failed: "+err.Error()) which incorrectly
classifies client-side validation/precondition failures; update the handlers in
internal/api/handler_marketplace.go to inspect the returned AWS error (check for
awserr.Error or smithy.APIError / error with Code() and/or HTTPStatusCode()
methods) and map its actual HTTP-level or SDK error code to an appropriate
status (e.g., 4xx for validation/precondition/auth errors, 409 for conflict, 404
for not found, 401/403 for auth/perm, otherwise 502 for server-side), then call
NewClientError with that mapped status and the error message; apply the same
change to the other occurrence that currently returns 502 so client-correctable
errors are propagated with the correct 4xx status.
- Around line 151-156: When UpdatePurchaseHistoryListing(...) fails after AWS
listing creation (the block using purchaseID, result.ListingID, result.State),
do not silently return success; instead treat it as an error: attempt a
compensating rollback of the created listing (call the appropriate marketplace
delete/cleanup method for result.ListingID), log both the DB error and the
rollback outcome, and return an internal error to the API caller; apply the same
change for the other occurrence referenced (the block around lines 207-209) so
DB persistence failures never yield a successful response while external state
exists.
- Around line 117-118: The default listing price uses row.Term as the remaining
months, which overprices older reservations; instead compute remainingMonths as
the actual remaining term (e.g., row.RemainingTerm or row.Term - row.UsedMonths,
or derive from start/end dates and now) and pass that into
resolveMarketplacePriceSchedule when body.PriceSchedule is empty; update the
code around the schedule assignment (the call to resolveMarketplacePriceSchedule
and the local variable schedule) to use this computed remainingMonths and apply
the same fix to the other occurrence referenced (lines ~299-310).

In
`@internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.up.sql`:
- Around line 15-18: The migration for table purchase_history adds
offering_class, listing_id, and listing_state but omits required marketplace
columns needed for lifecycle persistence; update the ALTER TABLE statement in
the migration to also ADD COLUMN IF NOT EXISTS listed_at TIMESTAMP WITH TIME
ZONE, listing_price_schedule JSONB (or appropriate type),
listing_proceeds_received NUMERIC (or appropriate money type), and
listing_fee_paid NUMERIC so the poller/sale path (references: table
purchase_history and columns offering_class, listing_id, listing_state) can
persist listing timestamps, price schedules, proceeds, and fees; ensure
nullability and default behavior match existing schema conventions.

In `@providers/aws/services/ec2/client.go`:
- Around line 970-979: Ensure the code never returns an empty ListingID: when
building MarketplaceListingResult (using variables listingID and
listing.ReservedInstancesListingId and listing.Status) validate that
aws.ToString(listing.ReservedInstancesListingId) yields a non-empty string and,
if empty, return a clear error instead of MarketplaceListingResult{ListingID:
"", ...}; apply the same validation & error-return logic in the other similar
blocks (the code paths referenced around the other two snippets) so downstream
handlers never persist an empty ListingID.

---

Outside diff comments:
In `@frontend/src/history.ts`:
- Around line 610-624: The early return when lineage.length > 0 prevents the
marketplace buttons from being rendered for completed retry descendants; update
the logic in the function handling the history row so that when lineage exists
you append (or merge) the marketplace button markup instead of returning
immediately — locate the lineage handling and the marketplace block using
lineage, p.purchase_id, canCancelMarketplaceListing, canSellOnMarketplace and
escapeHtml; either move the marketplace block above the return or build a
combined string (lineage.join(' ') + marketplaceMarkup) so Sell on Marketplace /
Cancel listing buttons are included for eligible purchases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b216fad4-4a77-4b3b-be24-d77f8f3b76d4

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and 8bab8ca.

📒 Files selected for processing (24)
  • frontend/src/api/index.ts
  • frontend/src/api/purchases.ts
  • frontend/src/history.ts
  • frontend/src/types.ts
  • internal/analytics/collector_test.go
  • internal/api/handler.go
  • internal/api/handler_marketplace.go
  • internal/api/mocks_test.go
  • internal/api/router.go
  • internal/auth/service_group_test.go
  • internal/auth/service_test.go
  • internal/auth/types.go
  • internal/auth/types_test.go
  • internal/config/interfaces.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/config/types.go
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.down.sql
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.up.sql
  • internal/mocks/stores.go
  • internal/purchase/mocks_test.go
  • internal/scheduler/scheduler_test.go
  • internal/server/test_helpers_test.go
  • providers/aws/services/ec2/client.go

Comment thread frontend/src/history.ts
Comment thread frontend/src/history.ts Outdated
Comment thread internal/api/handler_marketplace.go Outdated
Comment thread internal/api/handler_marketplace.go Outdated
Comment thread internal/api/handler_marketplace.go
Comment thread providers/aws/services/ec2/client.go
@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

Fixed the compile/type issues (missing MockEC2Client methods for the extended EC2API interface), addressed the 7 CR findings (6 were already in prior commits and verified against HEAD; the Describe/Cancel empty-listing-ID fallback is now applied), and resolved the pre-commit failures (gocyclo on marketplaceList via extracting validateMarketplaceListRequest, and the stale permissions.generated.ts codegen) in 03ab7f5. @coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

@cristim Thanks for the detailed summary of the fixes in 03ab7f574! Triggering a fresh incremental review now to cover the updated commit.

(ᴗ˳ᴗ)

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

All 8 CodeRabbit findings from the 2026-05-28 review are resolved in commits a3cfc89 to 03ab7f5. Verification summary:

frontend/src/history.ts

  • Remaining-term gate: canSellOnMarketplace computes elapsed months from purchase timestamp and returns false when fewer than 1 month remains (line 498-504).
  • Lineage + marketplace composability: trailing actions are built before the lineage join, so retry descendants with eligible listings still get Sell/Cancel buttons (line 624-635).
  • Pricing modal: sell handler builds a DOM-based modal showing RI summary, remaining term, list price (5% below residual value), 12% fee, and estimated net proceeds before calling api.createMarketplaceListing (lines 938-985).

internal/api/handler_marketplace.go

  • Remaining-months accuracy: computeRemainingMonths(row.Timestamp, row.Term) is used instead of raw row.Term so default price schedules reflect actual residual value.
  • AWS error mapping: mapAWSMarketplaceError inspects smithy.APIError fault + known client-fault codes and returns 4xx for client errors, 502 for AWS-side failures.
  • DB failure on listing create: compensating CancelMarketplaceListing rollback attempted; both DB error and rollback outcome are logged; internal error returned to caller instead of success.

internal/database/postgres/migrations/000060_...

  • Added listed_at TIMESTAMPTZ, listing_price_schedule JSONB, listing_proceeds_received NUMERIC, listing_fee_paid NUMERIC to the ALTER TABLE.

providers/aws/services/ec2/client.go

  • CreateReservedInstancesListing: errors on empty listing ID.
  • DescribeMarketplaceListing / CancelMarketplaceListing: falls back to caller-supplied ID when AWS omits it in the response.

Tests: go build ./... clean, go test ./internal/api/... 1355 passed, go test in ec2 submodule 39 passed, frontend Jest 179 passed.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

@cristim Thanks for the thorough verification summary — every finding is clearly mapped to the exact lines and commit range. The compensating rollback on DB failure, the AWS error mapping via mapAWSMarketplaceError, the remaining-months accuracy fix, and the extra migration columns all look well-considered. Triggering a fresh incremental review now.

(ᴗ˳ᴗ)

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

All 7 CR findings from the 2026-05-28 review (plus the outside-diff lineage composability note) are resolved and confirmed in the 2026-06-01T19:33 incremental review. No new findings were raised in that review pass. Verification summary:

7 inline threads - all confirmed LGTM by CR at 2026-06-01T19:33:

  • Thread 3319800619 (canSellOnMarketplace remaining-term gate): confirmed at 3336658526. Code at history.ts:498-504 computes elapsedMonths from purchase timestamp and returns false when remainingMonths < 1.
  • Thread 3319800649 (pricing modal for sell flow): confirmed at 3336658686. Sell handler builds a dl summary with RI ID, region, resource type, remaining term, default list price (residual value x 0.95), 12% AWS fee, and estimated net proceeds before calling createMarketplaceListing.
  • Thread 3319800653 (default listing price uses full-term cost): confirmed at 3336658817. computeRemainingMonths(row.Timestamp, row.Term) used in both marketplaceList and marketplaceCancel handlers.
  • Thread 3319800660 (AWS marketplace errors mapped to 502): confirmed LGTM at 3336659205. mapAWSMarketplaceError uses smithy.APIError.ErrorFault() == smithy.FaultClient as primary classifier, plus a deny-list of known client-fault codes.
  • Thread 3319800663 (success returned after failed DB persistence): confirmed at 3336659846. Compensating CancelMarketplaceListing call on UpdatePurchaseHistoryListing failure; both error paths return an internal error.
  • Thread 3319800664 (missing settlement columns in migration): confirmed at 3336660174. Migration 000060 now adds all 7 columns: offering_class, listing_id, listing_state, listed_at, listing_price_schedule, listing_proceeds_received, listing_fee_paid.
  • Thread 3319800673 (empty listing IDs returned from ec2 client): confirmed at 3335986569 and 3336658526. Create path errors on empty ID; Describe/Cancel paths fall back to caller-supplied listingID.

Outside-diff finding (lineage composability - history.ts:610-624): already fixed in the same commit range. trailingActions array is built before lineage join at history.ts:624-635 so retry descendants with eligible listings get Sell/Cancel buttons.

Build and tests pass: go build ./... clean, go test ./internal/api/... 1355 passed, go test ./providers/aws/services/ec2/... 39 passed.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

@cristim Thanks for the meticulous resolution log — every thread is mapped to a specific commit reference and line range, making it easy to verify. All 7 findings plus the outside-diff composability note are clearly closed out, and the clean build + test results across all three suites look solid. Triggering a fresh incremental review now.

(ᴗ˳ᴗ)

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 3, 2026

Rebased on feat/multicloud-web-frontend (18 base commits since last rebase).

Conflict resolution:

  • internal/auth/service_group_test.go (4 blocks): base migrated to group-membership-only authz (Revamp authorization: group-membership-only (remove roles), require >=1 group per user #907). Adopted base semantics: where the test setup includes a standard-group with DefaultUserPermissions(), the count is now 12 (standard-group, which already contains sell-own #292) + N (other groups) = 12+N. Where the test setup gives the user no standard-group or no groups at all, the empty-permission assertion from base is the correct outcome.
  • internal/auth/service_test.go (1 block): same shape. Sole-group-fails test now correctly asserts Empty(t, permissions) since there is no role-based fallback.
  • internal/api/handler_marketplace.go (rebase compile fix): session.Role == "admin" is gone in the group-only model; both authorizeSessionSell and authorizeAllowedAccount now check admin via h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionAdmin, auth.ResourceAll) matching the pattern used in middleware.go.

Verification:

  • gofmt -l . clean
  • go vet ./... clean
  • go build ./... clean
  • go test across all touched packages (internal/auth, internal/api, internal/config, internal/purchase, internal/scheduler, internal/analytics, internal/server, internal/mocks, providers/aws/services/ec2): 3292 passed, 0 failed
  • npx tsc --noEmit (frontend): clean
  • Frontend Jest history suite: 180 passed, 0 failed

All previous CR findings from the 2026-05-28 pass remain addressed (no logic changes to marketplace handlers, history.ts, ec2 client, or migration). The outside-diff-range note (history.ts:610-624 lineage gate) is also addressed by the existing commits: line 634 now returns [...lineage, ...trailingActions].join(' ') so the Sell/Cancel buttons render even on retry descendants.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

@cristim Thanks for the thorough rebase summary! A few things stand out as well-considered:

  • The session.Role == "admin"h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionAdmin, auth.ResourceAll) change in handler_marketplace.go is the right alignment with the group-membership-only authz model from #907 — it ensures marketplace sell/cancel authorization goes through the same permission-graph path as the rest of the middleware.
  • The internal/auth/service_group_test.go conflict resolution (treating standard-group as the permission carrier and asserting Empty for users with no group) correctly reflects the new semantics where role-based fallback no longer exists.

The clean build + 3292 tests passing + frontend Jest green across all touched packages is a solid confidence signal. Triggering a fresh incremental review now to cover the rebase commits.

(ᴗ˳ᴗ)


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 4, 2026

Rebased fix/292-wave4 onto the latest feat/multicloud-web-frontend (now includes the base build fix from #941 and #926). go build ./... succeeds; gofmt/vet clean; touched-package tests green.

The prior round's 7 CR findings were addressed in the branch history:

  • history.ts early-return suppressing marketplace actions for retry descendants -> fixed in 2e22d55 (frontend findings commit, replayed post-rebase)
  • handler_marketplace.go HTTP status classification (502 vs 4xx) -> fixed in the Go handler findings commit
  • handler_marketplace.go DB-persist-failure-after-AWS-listing rollback -> fixed in the Go handler findings commit
  • handler_marketplace.go remaining-months pricing (row.Term overpricing) -> fixed in the Go handler findings commit
  • migration 000060 missing settlement columns (listed_at, listing_price_schedule, listing_proceeds_received, listing_fee_paid) -> fixed in the settlement-columns migration commit
  • ec2/client.go empty ListingID guard -> fixed in the validate-non-empty-listing-ID commit
  • canSellOnMarketplace remaining-term gate + pricing modal -> addressed in the frontend findings commit

Note: internal/auth/TestLogin_WithMFA_NoSecret/wrong_password_returns_same_generic_error fails identically on the base branch itself (this PR touches neither internal/auth/service.go nor service_test.go), so it is a pre-existing base inconsistency, not a regression from this PR.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

@cristim Thanks for the detailed rebase summary and the clean verification signal. The mapping of all 7 prior findings to specific commits is clear, and the note about TestLogin_WithMFA_NoSecret/wrong_password_returns_same_generic_error being a pre-existing base inconsistency (untouched by this PR's file set) is well-reasoned. Triggering a fresh incremental review now to cover the rebase commits.

(ᴗ˳ᴗ)


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/api/handler_marketplace.go`:
- Around line 401-412: The default price schedule is using the full upfrontCost
instead of the prorated remaining upfront; update
resolveMarketplacePriceSchedule to accept the original term (e.g., add a
parameter originalTerm int64 or float64) and compute upfrontRemaining :=
upfrontCost * (float64(remainingMonths) / float64(originalTerm)), then set
totalValue := upfrontRemaining + (monthlyCost * float64(remainingMonths)) and
continue with listPrice := totalValue * 0.95 and clamping to >=0; also update
the call site in marketplaceList to pass row.Term into
resolveMarketplacePriceSchedule so the function has the original term available.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a47024a4-0465-406d-b1b1-370d84820e04

📥 Commits

Reviewing files that changed from the base of the PR and between 8bab8ca and 2330fc6.

📒 Files selected for processing (25)
  • frontend/src/api/index.ts
  • frontend/src/api/purchases.ts
  • frontend/src/history.ts
  • frontend/src/permissions.generated.ts
  • frontend/src/types.ts
  • internal/analytics/collector_test.go
  • internal/api/handler.go
  • internal/api/handler_marketplace.go
  • internal/api/mocks_test.go
  • internal/api/router.go
  • internal/auth/service_group_test.go
  • internal/auth/types.go
  • internal/auth/types_test.go
  • internal/config/interfaces.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/config/types.go
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.down.sql
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.up.sql
  • internal/mocks/stores.go
  • internal/purchase/mocks_test.go
  • internal/scheduler/scheduler_test.go
  • internal/server/test_helpers_test.go
  • providers/aws/services/ec2/client.go
  • providers/aws/services/ec2/client_test.go
💤 Files with no reviewable changes (15)
  • internal/auth/types_test.go
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.down.sql
  • providers/aws/services/ec2/client_test.go
  • internal/database/postgres/migrations/000060_purchase_history_marketplace_listing.up.sql
  • internal/server/test_helpers_test.go
  • internal/config/interfaces.go
  • internal/scheduler/scheduler_test.go
  • internal/purchase/mocks_test.go
  • internal/auth/service_group_test.go
  • internal/auth/types.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/mocks/stores.go
  • internal/config/types.go
  • providers/aws/services/ec2/client.go
  • internal/config/store_postgres.go
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/permissions.generated.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • internal/api/mocks_test.go
  • internal/api/router.go
  • internal/api/handler.go
  • frontend/src/types.ts
  • frontend/src/api/purchases.ts
  • frontend/src/history.ts
  • frontend/src/api/index.ts

Comment thread internal/api/handler_marketplace.go Outdated
@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 4, 2026

Pushed a follow-up fix: extracted scanPurchaseHistoryRow from queryPurchaseHistory in internal/config/store_postgres.go to bring the function back under the gocyclo budget of 10 (it had reached 11 after the nullable marketplace columns + nullable monthly_cost handling were added to the per-row scan). Behavior unchanged; gofmt/vet/build/config-tests green. This was the only CI failure on the rebased branch (the AWS/Azure Sanity and Frontend Build checks are green).

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

@cristim The scanPurchaseHistoryRow extraction is a clean approach to the gocyclo issue — pulling the per-row scan logic into its own function is both the idiomatic fix and keeps the diff minimal and behavior-preserving. Good call keeping it to a single helper rather than splitting further. Triggering an incremental review to cover this commit now.

(ᴗ˳ᴗ)


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 277
File: frontend/src/__tests__/recommendations.test.ts:2472-2476
Timestamp: 2026-05-05T07:46:01.902Z
Learning: In the CUDly frontend (`frontend/src/recommendations.ts`), `effectiveSavingsPct` intentionally has NO plausibility guard (no null return or warning for reconstructed percentages above per-term ceilings). The maintainer explicitly rejected that approach in PR `#277`. The fix for inflated percentages (issue `#274`) is purely a data-path fix: plumbing the provider's canonical `on_demand_cost` through to the frontend so the denominator is not reconstructed from potentially misleading `monthly_cost + savings + amortized`. Tests that pin the reconstructed >80% behavior are intentional documentation of the pre-fix path, not bugs.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-25T21:45:57.471Z
Learning: In this repository (CUDly), `internal/commitmentopts/normalizePayment` is unexported and AWS-spelling-focused (parses "All Upfront"/"ALL_UPFRONT" → canonical AWS tokens). `internal/api/validation.go:purchasePaymentWhitelist` is the purchase-execute boundary. `pkg/common/reservation_name.go:normalizeReservationPayment` produces short-form name segments. These are all distinct concerns from the cross-provider `NormalizePaymentOption` helper in `internal/config/validation.go`.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 4, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 4, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 5, 2026

Pushed 690e7f7 addressing the adversarial-review gaps. Per-gap disposition:

Gap 1 (BLOCKER) - compile against current base: RESOLVED.
Rebased onto origin/feat/multicloud-web-frontend (clean; the only base commit ahead touched cmd/ only). The interfaces.go GetPurchaseHistoryFiltered(PurchaseHistoryFilter) signature was already aligned with base on this branch, and both new store methods (GetPurchaseHistoryByPurchaseID, UpdatePurchaseHistoryListing) are retained. Migration 000060 is a free slot (base is at 000063/000064) - no collision, kept. Also repaired a stale Session.Role="admin" reference in a pre-existing purchases test (the field was removed in the group-only auth migration) so the internal/api test package compiles.

Gap 2 (BLOCKER) - multi-count partial-listing: FIXED.
CreateReservedInstancesListing no longer hardcodes InstanceCount=1. MarketplaceListingRequest gained an InstanceCount field (rejected when <= 0 before the outbound call); the handler passes row.Count (floored at 1 for legacy rows). A row of N Standard RIs now lists all N. Covered by TestClient_CreateMarketplaceListing_HappyPathMultiCount (asserts InstanceCount == 3) and TestMarketplaceList_SellOwnAllowed (asserts the outbound request carries the row's count).

Gap 3 (BLOCKER) - zero handler tests: ADDED.
internal/api/handler_marketplace_test.go covers: convertible offering_class -> 400; sell-own allow + deny (account scoping); duplicate active listing -> 409; AWS error mapping (client fault -> 400, server fault + unknown error -> 502); DB-failure compensating rollback (asserts the cancel fires); default-schedule proration math. EC2 client tests in client_test.go exercise Create/Describe/Cancel via the SDK mock (8 new cases), including the non-positive-count guard and empty-listing-ID guard.

Gap 4 (SCOPE) - dead poller columns: DESCOPED.
Migration 000060 now adds only offering_class, listing_id, listing_state - the full set the implemented list/cancel flow reads/writes. The four columns nothing populates (listed_at, listing_price_schedule, listing_proceeds_received, listing_fee_paid) were removed so the schema never carries unpopulated columns. Filed follow-up #966 for the status poller (active->closed + proceeds/fee) and seller-account precondition UI (the #292 remainder). PR body updated to state the descope.

Verification: gofmt -l clean, go vet ./... clean, go build ./... ok, go test ./internal/api/... -count=1 -> 1469 pass, go test ./providers/aws/services/ec2/... -count=1 -> 47 pass, SKIP=terraform_validate pre-commit run on the touched files passes (gocyclo + migration-conflict green). No frontend files touched.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 5, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 5, 2026

Rebased on feat/multicloud-web-frontend. Resolved conflicts in:

  • internal/api/handler_purchases_test.go (additive: removed stale Session.Role comment, kept grantAdmin call)
  • providers/aws/services/ec2/client_test.go (additive: merged NameTag tests from base with marketplace listing tests from this PR)

All prior CR findings were already addressed. Requesting fresh review pass.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

@cristim Thanks for the detailed rebase notes. The two conflict resolutions look clean and low-risk — the Session.Role removal is consistent with the group-only authz model from #907, and the client_test.go merge is purely additive. Triggering a fresh incremental review now to cover the rebase commits.

(ᴗ˳ᴗ)


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-05T08:48:15.584Z
Learning: In LeanerCloud/CUDly PR `#804`, `callAzureReturn` in `internal/api/handler_purchases_revoke.go` rejects an order-only ARM path (empty `reservationID`) with a 422 instead of submitting an empty Return call to Azure.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim added 12 commits June 5, 2026 14:08
…end (closes #292)

Add sell-on-Marketplace support for Standard Reserved Instances:

- migration 000060: add offering_class, listing_id, listing_state to purchase_history
- auth: sell-any and sell-own actions; sell-own added to DefaultUserPermissions
- config: extend PurchaseHistoryRecord, StoreInterface with GetPurchaseHistoryByPurchaseID + UpdatePurchaseHistoryListing
- ec2 client: CreateReservedInstancesListing, DescribeReservedInstancesListings, CancelReservedInstancesListing
- api: handler_marketplace.go with marketplaceList/Cancel, authorizeSessionSell (sell-any/sell-own RBAC), default price schedule (95% of residual value)
- router: POST /api/purchases/{id}/marketplace-list + /marketplace-cancel routes
- all tests updated for the new interface methods and permission count
…tend (closes #292)

Add Sell on Marketplace / Cancel listing buttons to purchase history rows:

- types.ts: extend HistoryPurchase with offering_class, listing_id, listing_state
- api/purchases.ts: createMarketplaceListing + cancelMarketplaceListing; re-export from api/index.ts
- history.ts: canSellOnMarketplace / canCancelMarketplaceListing predicates; renderActionCell renders Sell/Cancel buttons for Standard RI completed rows; wireRowActionHandlers wires confirm-dialog + API + toast + reload for both buttons
- canSellOnMarketplace: gate on remaining term >= 1 month computed from
  purchase timestamp + total term; matured Standard RIs no longer show Sell
- sell click handler: open pricing/schedule modal before calling
  createMarketplaceListing so users see RI summary, default list price, and
  12% fee breakdown before confirming
- lineage actions cell: build trailingActions[] first then combine with
  lineage[] so Sell/Cancel buttons render on retry-descendant rows
- Compute actual remaining months from purchase timestamp and total term
  via computeRemainingMonths; removes overpricing of older RIs in the
  default price schedule (was using full contract term)
- Map AWS errors via mapAWSMarketplaceError: inspect smithy.APIError code
  and fault; client-fault errors return 4xx instead of blanket 502
- On DB failure after successful listing creation, attempt compensating
  rollback via CancelMarketplaceListing to prevent AWS/DB desync; return
  internal error rather than false success to the caller
- Same DB-failure fix for cancel handler: return error on UpdatePurchaseHistoryListing
  failure so the caller knows the state is out of sync
Add listed_at, listing_price_schedule, listing_proceeds_received, and
listing_fee_paid columns for marketplace settlement tracking. All nullable,
consistent with existing offering_class/listing_id/listing_state nullability.
Down migration updated to drop the new columns.
…isting

Return an error when AWS returns a listing with an empty
ReservedInstancesListingId rather than propagating a blank ID that would
silently make rollback and describe calls fail.
Repair the stalled marketplace sell/cancel work:

- Add the three missing marketplace methods to MockEC2Client so the
  ec2 package compiles against the extended EC2API interface
  (CreateReservedInstancesListing, DescribeReservedInstancesListings,
  CancelReservedInstancesListing).
- Extract validateMarketplaceListRequest from marketplaceList to bring
  its cyclomatic complexity back under the gocyclo threshold.
- Regenerate frontend/src/permissions.generated.ts to include the new
  sell-own:purchases permission (pre-commit codegen check).
- Harden Describe/Cancel marketplace paths to fall back to the
  caller-supplied listing ID when AWS omits ReservedInstancesListingId,
  so downstream persistence never stores an empty ID (CR finding).
After rebasing onto feat/multicloud-web-frontend, base PR #258 made
PurchaseHistoryRecord.MonthlyCost a *float64 (nullable). The marketplace
default price-schedule path passed it as a float64. Dereference it
nil-safely at the call site, treating an absent monthly breakdown as a
zero recurring contribution to residual value (per the nullable-not-zero
convention: nil means "no breakdown recorded", which contributes nothing
to the residual list price).
queryPurchaseHistory reached cyclomatic complexity 11 (budget 10) after the
nullable marketplace columns (offering_class, listing_id, listing_state) and
the nullable monthly_cost handling were added to the per-row scan. Pull the
scan plus nullable-column reconciliation into scanPurchaseHistoryRow so the
parent function drops back under the limit. Behavior is unchanged.
…lt schedule

When the caller omits a price schedule, the default was computing
total value as (full_upfront + recurring * remaining_months).
For an older RI this overprices the upfront component because
only (remaining/original) of the upfront has not yet been
amortized. Change to:

  upfront_remaining = upfront_cost * (remaining / original_term)
  total_value = upfront_remaining + recurring * remaining_months

Pass row.Term as originalTerm through resolveMarketplacePriceSchedule.
Addresses CR finding on handler_marketplace.go.
…columns (closes #292 partial)

Address adversarial-review gaps on the RI Marketplace sell/cancel PR:

- Multi-count fix: CreateMarketplaceListing now lists every RI in the row
  (purchase_history.count) instead of a hardcoded InstanceCount=1, so a row
  of N Standard RIs lists all N. The EC2 client rejects a non-positive count
  before the outbound call; the handler passes row.Count (floored at 1 for
  legacy rows). Adds EC2 client tests for create (multi-count + guards),
  describe, and cancel via the SDK mock.

- Tests: add handler_marketplace_test.go covering convertible -> 400,
  sell-own allow + deny (account scoping), duplicate active listing -> 409,
  AWS error mapping (client fault -> 400, server/unknown -> 502), DB-failure
  compensating rollback, and default-schedule proration math.

- Descope poller: migration 000060 keeps only the columns the implemented
  flow reads/writes (offering_class, listing_id, listing_state). The
  settlement/poller columns (listed_at, listing_price_schedule,
  listing_proceeds_received, listing_fee_paid) are removed so the schema
  never carries columns nothing populates; they land with the poller in the
  #292 follow-up (#966).

- Repair a stale Session.Role="admin" reference in a pre-existing purchases
  test (removed in the group-only auth migration) so the api test package
  compiles after rebasing onto the current base.
000060 is taken by 000060_cleanup_universal_plans on base.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p2 Backlog-worthy severity/low Minor harm triaged Item has been triaged type/feat New capability urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant