[IAmJSD]

Root cause

Before this PR, mapAWSMarketplaceError handled AWS's SellerNotRegistered the same way as every other client-fault code: a 400 carrying AWS's own message. That text only says the account isn't a registered seller. It doesn't tell the user how to fix it, and there was nothing a client could branch on to show its own guidance.

Fix

mapAWSMarketplaceError (internal/api/handler_marketplace.go) now handles SellerNotRegistered before the generic client-fault check. It returns a 400 with:

• a fixed error message: the account's root user must register it as a Reserved Instance Marketplace seller, with a bank account that has a US address, before it can list. The message links the AWS guide at https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ri-market-general.html#ri-market-seller-profile.
• details.reason = "seller_not_registered". The response writer flattens it into the JSON body: {"error": "...", "reason": "seller_not_registered"}.

Behavior notes:

• Matching: the branch keys on the error code alone, so it applies whatever fault AWS reports.
• AWS's own text is dropped: the fixed message replaces it.
• Status stays 400: isClientFaultError therefore still drops the attempt's client token, exactly as before.
• STS AssumeRole failures are still classified first and are unaffected.
• Other client faults keep AWS's message and carry no reason.
• Fault-code set: I removed SellerNotRegistered from awsMarketplaceClientFaultCodes, because the new branch makes that entry unreachable. A comment on the set says so.

internal/api/openapi.yaml gets a new MarketplaceClientError schema (error, plus an optional reason with enum [seller_not_registered]). It's used on the 400 for both marketplace-list and marketplace-cancel, since both go through the same mapping, and the 400 descriptions now explain the reason.

There's no DB change and no migration. The change only maps an error that has already happened, so concurrency doesn't come into it.

Refs #27

Verification

Unit and mock evidence only. No real AWS calls were made.

Fail-before: the message constant was in place but the new branch wasn't.

go test -count=1 -run 'SellerNotRegistered|OtherClientFaultsHaveNoReason' ./internal/api/
--- FAIL: TestMarketplaceListHTTP_SellerNotRegisteredExplainsRegistration
    expected: {"error":"this AWS account is not registered as a Reserved Instance Marketplace seller. ...", "reason":"seller_not_registered"}
    actual  : {"error":"raw aws text"}
--- FAIL: TestMapAWSMarketplaceError_SellerNotRegistered   (client and unknown fault)
    expected: map[reason:seller_not_registered]  actual: map[](nil)
FAIL

Pass-after:

--- PASS: TestMarketplaceListHTTP_SellerNotRegisteredExplainsRegistration
--- PASS: TestMapAWSMarketplaceError_SellerNotRegistered (/client, /unknown)
--- PASS: TestMapAWSMarketplaceError_OtherClientFaultsHaveNoReason

What the tests cover:

• TestMarketplaceListHTTP_SellerNotRegisteredExplainsRegistration drives POST /marketplace-list through HandleRequest against the fake listing DB. It asserts:
  • the exact JSON body (error plus reason, nothing else);
  • the attempt token is dropped and no listing state is left behind.
• TestMapAWSMarketplaceError_SellerNotRegistered wraps the code in an EC2 smithy.OperationError with both a client fault and an unknown fault. It asserts:
  • status 400;
  • the fixed message, which mentions US and contains the doc URL;
  • AWS's raw text does not leak;
  • Details() is exactly {reason: seller_not_registered}.
• TestMapAWSMarketplaceError_OtherClientFaultsHaveNoReason pins that InvalidParameterValue keeps its AWS message and has nil details. It's a guard against a broader mutation, so it passes both before and after.

Other checks:

go build ./... ; go vet ./... ; go vet -tags integration ./...           # ok
go test -count=1 ./internal/api/                                          # ok (31.7s), includes the openapi_* spec tests
go test -count=1 ./internal/mocks/                                        # ok
golangci-lint v2.10.1 run --allow-parallel-runners ./internal/api/        # 0 issues
gocyclo -over 10 -ignore "_test\.go" internal/api/handler_marketplace.go  # clean (mapAWSMarketplaceError = 6)

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ 400d256b = origin/main 7b86e386 plus these heads, merged in this order:

Branch	Tested head
fix/issue-243-rest	007c163
fix/issue-1206	312072f
fix/issue-111-email	6d422e2
fix/issue-111-fail-closed	3817e29
fix/issue-712-age-cap	a91120b
fix/issue-712-fargate	aeaf0d4
fix/issue-613-legacy-ids	350e837
fix/issue-120-guards	6f38ebc
fix/issue-120-shared-predicate	cb25735
fix/issue-151-account-update-keep	d20cf01
fix/issue-713-upcoming-filter	e73fdfb
fix/issue-263-bastion-opts	7861f32
fix/issue-43-fixture-ids	8cee203
fix/issue-177-makefile-walk	365a9d6
fix/issue-27-seller-not-registered	d845f66
fix/issue-313-user-columns	6d759dd
fix/issue-353-mock-panics	aac2edf
fix/issue-8-config-stamps	b548ae5
fix/issue-30-monthly-totals	f637921

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. The full suite ran on f7c48020, which differs from this tree only by test-only commits on fix/issue-30, fix/issue-713 and fix/issue-263 that replace placeholder account-number literals with named constants:

• 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, every changed package that has them, one package at a time (-tags integration -count=1): internal/analytics, internal/api, internal/auth, internal/config, internal/database/postgres/migrations, internal/purchase, internal/scheduler and internal/server all pass. Migrations 000149, 000150 and 000151 apply in order on this tree.
• Frontend: the changed jest files (approval-details, settings-accounts, api) pass (221 tests).
• Secrets: the first run flagged 9 new fixture lines (placeholder account numbers next to an account key); fixed with named constants. Re-run on this tree: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.
• Terraform .tftest.hcl files were not run locally (CI runs them). Nothing was applied to any cloud.

Out of scope

• Proceeds in the API (D23).
• Superseded listings (D24).
• The Sell dialog UI, including a check before listing. This PR only explains the failure after AWS rejects the listing. The issue's "precondition check before listing" stays open, so this PR uses Refs.

Frontend follow-up for the UI owner

The current UI already shows the new error text through apiRequest's message handling, so users see the guidance without any frontend change. The URL appears as plain text, though. apiRequest puts the rest of the JSON body on error.details, so the Sell flow (frontend/src/history.ts, around api.createMarketplaceListing) could branch on error.details.reason === 'seller_not_registered' to:

• render the guidance with a clickable link to the AWS seller-registration guide;
• tell the user that retrying won't help until the account is registered.

Maintainer: please mirror the issue's triage labels onto this PR: effort/l, impact/few, priority/p2, severity/low, triaged, type/feat, urgency/eventually.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 58 minutes.

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: 7b86e38
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 54b8a7b7-ec3e-4357-9c54-4dde999f691b

Commits

Reviewing files that changed from the base of the PR and between 7b86e38 and 7b7a77d.

Files ignored due to path filters: 1

• go.sum is excluded by !**/*.sum

Files selected for processing: 5

• go.mod
• internal/api/handler_marketplace.go
• internal/api/handler_marketplace_attempt_test.go
• internal/api/handler_marketplace_credentials_test.go
• internal/api/openapi.yaml

• 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 7b7a77db905f71f03dd998e4a721b7ee30c7107d

Blocking:

• none

Non-blocking:

• internal/api/handler_marketplace.go:488: the branch keys on the literal "SellerNotRegistered", an error code carried over from main's client-fault set. Nothing in the repo (test or SDK constant) shows that EC2 CreateReservedInstancesListing really returns that exact code. If AWS uses a different code for an unregistered seller, the user still gets the generic 400 with AWS's text, so nothing gets worse. It's just worth confirming the code once from a real response or the EC2 error-code reference.
• The fixed message replaces AWS's text entirely. If AWS ever adds detail to this error, such as which registration step is missing, the user won't see it. You could append AWS's message after the guidance, or log it at Info. This is minor.
• MarketplaceClientError.reason on marketplace-cancel: a cancel can't realistically hit SellerNotRegistered. Documenting it there is accurate, since both share the mapping, just mostly theoretical.

Attacked:

• Ordering: the STS role-assumption classification still runs first, so an AssumeRole failure can't be mislabeled. The new branch comes before the fault-set check and covers both client and unknown faults, which the test pins. A 400 still means isClientFaultError drops the attempt token, and the HTTP test asserts no token and no listing state remain, so there's no stuck claim and no double listing on retry. No raw AWS text leaks.
• Checked that no other marketplace 400 carries details.reason, so the openapi claim "no other 400 carries a reason" holds, and that NewClientErrorWithDetails flattens reason into the top-level body as the test asserts.
• Fail-before: I removed only the new SellerNotRegistered branch and kept the constants. TestMarketplaceListHTTP_SellerNotRegisteredExplainsRegistration and both TestMapAWSMarketplaceError_SellerNotRegistered subtests FAIL. I restored the file afterwards.
• Ran on d845f660: go build ./...; go vet with and without -tags integration on internal/api; golangci-lint v2.10.1 with and without --build-tags integration (0 issues); gocyclo -over 10 handler_marketplace.go (silent); go test -race -short -count=1 ./internal/api/ (ok); go test -race -tags integration -count=1 ./internal/api/ (ok, 181s, real Postgres).
• The head moved to 7b7a77db while I was reviewing. The only new commit is the x/net v0.62.0 bump, byte-identical to fix(deps): bump golang.org/x/net to v0.62.0 (GO-2026-6860, GO-2026-6866) #1564. On 7b7a77db I re-ran go build ./... and go vet ./..., both clean.

Body-stale:

• none

_{Reviewer: rG1. 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 7b7a77db905f71f03dd998e4a721b7ee30c7107d merged onto main (d05a1726), together with #1547, #1555, #1558, #1559, #1561, #1562 in that order (combined tree eea0132d, 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 7b7a77db, green CI, and the combined-tree verification above. Squash-merged at exactly 7b7a77db905f71f03dd998e4a721b7ee30c7107d.