[IAmJSD]

Summary

The issue offers "warn or block". This PR does the conservative version it suggests as the safe default:

• Onboarding: warn, don't block. aws-cn accounts are still accepted, so no existing account is stranded. Account responses now include a visible unsupported_partition notice.
• Purchases: block Savings Plans only. A Savings Plans purchase for an aws-cn account is refused with an error wrapping savingsplans.ErrUnsupportedPartition, before any provider is built or AWS is called.

Root cause

• go.mod pinned cloud-commitments-go/providers/aws at aeb8a6f (2026-10-19). That predates cloud-commitments-go#566, so the library had no aws-cn refusal.
• The platform derived nothing from an account's partition. awsRoleARNRegex accepts arn:aws-cn: role ARNs, and the account API never said Savings Plans are unsupported there.
• The library guard (after chore(deps): remove the compression override once serve depends on compression >= 1.8.2 #566) only fires when a region it sees is cn-*. A Savings Plans rec can carry no region, so the library alone cannot reliably catch these accounts. The account's role ARN can.

Fix (3 commits, each under ~400 lines)

1. chore(deps): bumps providers/aws to 92722e6 (the chore(deps): remove the compression override once serve depends on compression >= 1.8.2 #566 merge commit). MVS moves pkg from 541e05e to ff6225d. This is the minimal bump that includes chore(deps): remove the compression override once serve depends on compression >= 1.8.2 #566. It deliberately stops before cloud-commitments-go#587/docs(api): reconcile RI-exchange approve and reject specs with observed behavior #603/fix(plans): a plan with no matching recommendations schedules zero-count purchases instead of failing #609, which rename or remove API.
2. fix(accounts):
   • Adds config.AWSPartitionFromARN (the existing parseArnPartition now delegates to it), CloudAccount.AWSPartition() (from the role ARN, AWS accounts only) and config.AWSPartitionNotices.
   • CloudAccount gets a derived, non-persisted notices field (omitempty, no DB or migration change).
   • It is filled on create, update, get, list and create-self. For the self-account, which has no role ARN, the list and create-self paths use CUDly's own partition from the STS identity those paths already resolve. No new STS calls were added: get, create and update use only the role ARN.
3. fix(purchase):
   • rejectUnsupportedPartition runs per rec inside the processPurchaseRecommendations fan-out, before executeSinglePurchase builds the provider.
   • It applies to every Savings Plans slug (common.IsSavingsPlan(mapServiceType(...))) on an aws-cn account. Other recs (e.g. RIs) are unaffected, so a mixed run still ends partially_completed per fix(settings): a lingering inline range error stays visible after Reset settings or a reload because programmatic value sets do not re-run the validator #642.
   • The partition is passed in from executeForAccount (plan fan-out) and from resolveSingleAccountProvider (direct-execute and single-account). That function now returns a small purchaseTarget{externalID, awsPartition} in place of the bare ExternalID.
   • User-facing text, stored on the rec's error and the execution's error: savings plans purchase refused for this AWS China (aws-cn) account: savings plans are not supported in the aws-cn partition: the purchase path is USD-only.

Closes #1194

Verification

All evidence below is unit, mock or fixture evidence. Nothing calls AWS: tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because this machine's default AWS profile is invalid.

Fail-before / pass-after. I temporarily disabled the guard and made annotateAccountNotices a no-op, then restored both:

go test ./internal/purchase/ -run 'TestRejectUnsupportedPartition|AWSChina' -count=1
go test ./internal/api/ -run 'PartitionNotice|AWSChinaRoleARN|CommercialRoleARN' -count=1

• Before:
  • FAIL: TestRejectUnsupportedPartition, TestExecuteForAccount_AWSChinaSavingsPlanRefusedBeforeAnyAWSCall, TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused
  • FAIL: TestCreateAccount_AWSChinaRoleARN_ReturnsUnsupportedPartitionNotice, TestUpdateAccount_..., TestGetAccount_..., TestListAccounts_PartitionNotices
• After: all PASS. The controls also pass: TestCreateAccount_CommercialRoleARN_HasNoNotice, TestParseArnPartition, and the commercial, GovCloud and unknown-partition cases.

What the tests check:

• The fan-out test asserts CreateAndValidateProvider is never called for the aws-cn Savings Plans rec. The provider factory mock records calls and fails them.
• The direct-execute test runs an aws-cn Savings Plans rec next to an EC2 rec. Only the EC2 rec reaches the provider (exactly 1 factory call, and GetServiceClient is never called for the Savings Plans type).
• TestPinnedSavingsPlansClientRefusesAWSCN builds a real pinned savingsplans.Client for cn-north-1. It uses an HTTP client that counts requests and fails them all, so there is no network access. The test asserts ErrUnsupportedPartition and zero requests.
  • At the old pin the package does not compile: undefined: savingsplans.ErrUnsupportedPartition.

CI-equivalent checks:

• go build ./... and go vet ./internal/api/ ./internal/purchase/ ./internal/config/
• go test <pkg> -count=1 for every package in the module, one at a time: all ok, including internal/mocks.
• golangci-lint v2.10.1 with --allow-parallel-runners on ./internal/api/... ./internal/purchase/... ./internal/config/... (the integration build tag comes from .golangci.yml): 0 issues.
• gocyclo -over 10 -ignore "_test\.go" on the changed files: silent.
• scripts/gosec-hook.sh on the changed packages: rc=0.
• go mod tidy: no further diff.
• git diff --check: clean.
• git-secrets: ran on the changed files in a fresh temp repo with an isolated HOME. The clean scan passed, and a planted AKIA key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Real-cloud gap. This has not been run against a real aws-cn account or a CUDly deployment hosted in aws-cn.

Out of scope / notes

• Accounts with no role ARN. For access_keys accounts, or a self-account on ambient credentials whose host partition is unknown, the platform cannot tell the partition without calling AWS. For those accounts:
  • no onboarding notice is shown;
  • purchases rely on the library's region guard (cn-* client, rec or plan-details region).
• HTTP status. Synchronous execute and approve calls that fail this way still return the existing 502 "execution … failed: …". The message now names the aws-cn refusal, but the status is not mapped to a 4xx. Per-rec errors are aggregated as strings (purchaseErrors []string), so errors.Is can't see the sentinel at executeFailureError. Mapping it would mean carrying error values through the aggregation, which is a broader change.
• Recommendations. The notice mentions that recommendations may be unavailable (Cost Explorer through us-east-1, per the issue). This PR does not change how recommendations are collected.
• Other onboarding paths. Federation self-registration and org discovery don't return the notice in their own responses. The accounts then show it in the account list and detail views.
• Library commits pulled in. The bump also brings every providers/aws and pkg commit between the old and new pins, e.g. cloud-commitments-go#529 (skip non-USD recs at ingestion), fix(db): record the cancel actor on UI-initiated cancels #533, fix(ladder): stop netting elapsed scheduled tranches from the gap #535 (RI account scope, default unchanged) and fix(api): invalid or expired approval tokens and a missing revoke execution answer 500 instead of 4xx #564. The full platform unit suite passes on them.
• Known unrelated CI failure: govulncheck GO-2026-6809 (fixed by fix(deps): bump Go to 1.26.10 for GO-2026-6809 #1236).

Frontend follow-up for the UI owner

• Notice. GET/POST/PUT /api/accounts[...] and POST /api/accounts/self now return notices: [{code, message}] when relevant (omitted otherwise). Suggested UI:
  • show each notice as a warning banner on the account detail and edit views, and as a badge or tooltip in the accounts list;
  • show the warning right after creating an aws-cn account;
  • code unsupported_partition means Savings Plans purchases are refused for that account.
• Hide Savings Plans actions. Optionally, hide or disable Savings Plans purchase actions for accounts carrying that notice.

Labels

The issue currently has no triage labels (gh issue view 1194 --json labels returns []), so there is nothing to mirror. Could a maintainer triage #1194 and mirror its labels onto this PR?

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 14 billable files and costs up to $3.50.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: eb165b4
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 859aa969-0a5b-49a1-8f43-197e3c5c2dbd

Commits

Reviewing files that changed from the base of the PR and between eb165b4 and f28e89c.

Files ignored due to path filters: 1

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

Files selected for processing: 14

• go.mod
• internal/api/handler.go
• internal/api/handler_accounts.go
• internal/api/handler_accounts_partition_notice_test.go
• internal/config/aws_partition.go
• internal/config/aws_partition_test.go
• internal/config/types.go
• internal/purchase/azure_savings_plans_alias_proof_test.go
• internal/purchase/execution.go
• internal/purchase/execution_test.go
• internal/purchase/money_path_regression_test.go
• internal/purchase/partition_guard.go
• internal/purchase/partition_guard_test.go
• internal/purchase/plan_step_proofs_test.go

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

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

[IAmJSD]

CI fix pushed. New head: 8421f033.

Root cause. git-secrets' AWS account-ID detector (--register-aws) matched AccountID: "111111111111" inside the one-line sourceIdentity{...} literal in internal/api/handler_accounts_partition_notice_test.go. The .gitallowed placeholder entry for test files only covers that key/value when it is alone on its line, so the pre-commit hook failed and so did the "file-argument scan of all tracked files" case in scripts/test-git-secrets-allowlist.sh (136/137).

Fix. Commit ae981951 splits the literal so each field is on its own line. AccountID: "111111111111", now matches the existing allowlist entry. The value and the assertions are unchanged, and .gitallowed is untouched. I also merged origin/main into the branch twice, as plain merges with no conflicts.

Checks run locally on the new head:

• go build ./..., go vet ./..., go vet -tags integration ./... and the tests/e2e vet all pass.
• golangci-lint v2.10.1 reports 0 issues on internal/api, internal/config and internal/purchase, both with and without --build-tags integration.
• gocyclo -over 10 on the changed non-test files reports nothing.
• go test -race -short passes on all three packages.
• go test -race -tags integration passes on internal/config and internal/purchase. On internal/api, one test (TestIntegration_HistorySummary_TotalsCoverAllInScopeRows) failed on the first run because the overloaded host could not start its Postgres container within 30s. It passed on a rerun.
• The secrets scan of the changed files prints "clean; planted key flagged".
• bash scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

[IAmJSD]

Merged current main into the branch, which picks up #1418's fix for the main breakage behind the earlier red CI. No files conflicted. New head: f28e89c.

The merge did break the build in one place, fixed in a follow-up commit. Main's #1305 added internal/purchase/azure_savings_plans_alias_proof_test.go, which calls processPurchaseRecommendations with the old five-argument signature. This branch adds the awsPartition argument, so the purchase test package no longer compiled. The test's account is Azure, so the call now passes "", like the branch's other non-AWS call sites.

Checks:

• go build and go vet (default, integration, e2e) pass. golangci-lint reports 0 issues with and without the integration tag, and gocyclo is silent.
• go test -race -short and go test -race -tags integration pass on api, config and purchase.
• go mod verify passes, and go mod tidy -diff is clean.
• The git-secrets scan is clean, and the allowlist test gives 137 PASS.

[IAmJSD]

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

Blocking:

• none

Non-blocking:

• go.mod:87: through MVS, the new providers/aws@92722e6 pins pkg at ff6225d5ff04. That commit is on the unmerged branch of cloud-commitments-go#564 and is not an ancestor of go's main: GitHub compare main...ff6225d shows it 3 commits ahead. Its pkg/ tree differs from what fix(api): invalid or expired approval tokens and a missing revoke execution answer 500 instead of 4xx #564 actually merged (0dc660d/92722e6). The behavioral difference is common.RDSSizeFlexibleEngine: the branch treats Oracle BYOL as size-flexible, and the merged fix(api): invalid or expired approval tokens and a missing revoke execution answer 500 instead of 4xx #564 deliberately reverted that because it "would let a duplicate through". I checked that this can't reach the platform. go list -deps ./cmd/server ./cmd/lambda doesn't contain pkg/recfilter, and RDSSizeFlexibleEngine isn't referenced by providers/aws@92722e6 or by platform code. Every other pkg change since the old pin 541e05e is from go main. Builds stay reproducible through proxy.golang.org/sum.golang.org. Still, I'd add an explicit require github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020071945-92722e673741 so the platform pins a reviewed main commit and not a stale PR-branch snapshot. The real fix is in go: providers/aws's own go.mod pins the branch commit.
• internal/purchase/partition_guard.go:22: aws-cn accounts without a role ARN (access_keys, or a self-account on ambient credentials) rely only on the library's cn-* region guard, as the body says. That's acceptable, but it means "refused without calling AWS" holds only for role-ARN accounts.
• A refused purchase still surfaces as a 502 on the synchronous execute/approve paths (body "Out of scope" says the same). Issue item 3 asks for "a clear user-facing error". The message is clear, but the status isn't a 4xx. That could be a follow-up.

Attacked:

• Money path: executeSinglePurchase is the only caller of PurchaseCommitment, and every route reaches it through processPurchaseRecommendations, where the guard runs per rec before provider construction. Both the single-account path (resolveSingleAccountProvider -> purchaseTarget.awsPartition) and the plan fan-out path (account.AWSPartition()) pass the partition. A refused rec goes through the normal v.err aggregation: it records Error, Purchased stays false and no history is written, so nothing is miscounted as bought and RIs in the same run are unaffected. AWSPartitionFromARN matches the old parseArnPartition on edge inputs (arn:, arn::x, arn:aws).
• Fail-before: forcing rejectUnsupportedPartition to always return nil makes TestRejectUnsupportedPartition, TestExecuteForAccount_AWSChinaSavingsPlanRefusedBeforeAnyAWSCall and TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused FAIL. Restored afterwards.
• Library bump: I walked aeb8a6f..92722e6 (42 commits, all on go main) and 541e05e..ff6225d for pkg (see above). No downgrade: both old pins are ancestors of the new ones.
• Ran: go build ./...; go vet ./..., -tags integration ./... and e2e vet; the full go test -race -short -count=1 ./... (all ok, needed because of the bump); go test -race -tags integration -count=1 on internal/purchase, internal/config and internal/api (ok: 74s, 165s, 148s); golangci-lint v2.10.1 with and without --build-tags integration (0 issues); gocyclo -over 10 (clean); go mod tidy (no diff). CI is green on this head.

Body-stale:

• "Known unrelated CI failure: govulncheck GO-2026-6809 (fixed by fix(deps): bump Go to 1.26.10 for GO-2026-6809 #1236)": fix(deps): bump Go to 1.26.10 for GO-2026-6809 #1236 merged on 2026-10-24, so this no longer applies.

_{Reviewer: rB21. 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 f28e89c1fcc669a176fdeefabfbed27d2ef1a2ae

Blocking:

• none

Non-blocking:

• PR body / issue scope: fix(onboarding): warn or block aws-cn accounts that cannot buy Savings Plans #1194's done-when is "an aws-cn account shows the notice". This PR adds the notice to the API (notices on GET/POST/PUT /api/accounts[...]) but nothing in frontend/ renders it, and Savings Plans actions are not hidden. Users of the web UI still see nothing until the UI follow-up lands. Either file that follow-up as an issue and link it from fix(onboarding): warn or block aws-cn accounts that cannot buy Savings Plans #1194, or change Closes #1194 to Refs #1194.
• internal/api/handler_accounts.go (getAccount/updateAccount): these call annotateAccountNotices(account, ""), so a self-account on an aws-cn host (no role ARN) shows the notice in the list view and on create-self, but not on its detail or update response. This is a small inconsistency. The purchase side has the same gap (the self-account and access_keys accounts fall back to the library's cn-* region guard), and the body discloses it.
• go.mod: the minimal bump that includes cloud-commitments-go#566 also pulls in about 20 other library commits that change money-path behaviour, among them fix(auth): invalidate reset tokens on credential change and enforce expiry in SQL #562 (reshape targets capped at owned units), sec(api): remaining tenant-scoping gaps after #499 #520 (fail loud on an unparseable SP on-demand spend), fix(approval): email-link approvers without view:purchases cannot approve, and the retry toast is wrong for permanent errors #529 (non-USD recs skipped at ingestion) and fix(ladder): stop netting elapsed scheduled tranches from the gap #535. The whole module's -short suite passes on them, but they should be in the release notes. Other open PRs that bump providers/aws/pkg (e.g. feat(ri-exchange): pass offered instance types to reshape analysis #1246) will conflict on go.mod/go.sum, so merge order matters.
• The synchronous execute/approve path still returns 502 for this refusal. The message is clear and the body says so. A 4xx mapping would be cleaner later.

Attacked:

• Checked that processPurchaseRecommendations is the only caller of executeSinglePurchase, so the guard covers the plan fan-out (executeForAccount), direct-execute and single-account paths. The refusal goes through the normal per-rec error aggregation, so a mixed SP+RI run ends partially_completed and nothing is double-bought or marked purchased. The idempotency token derivation is untouched. AWSPartitionFromARN matches the old parseArnPartition on edge cases (arn::, no colon, unknown partition). Notices is response-only (omitempty, overwritten on every response, never written to the DB).
• Fail-before: I disabled the rejectUnsupportedPartition call in execution.go and the acct.Notices = assignment in handler_accounts.go. Then TestExecuteForAccount_AWSChinaSavingsPlanRefusedBeforeAnyAWSCall, TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused, and the Create/Update/Get/List notice tests failed. Restoring both made them pass.
• Ran go build ./..., go vet ./..., go vet -tags integration ./..., go test -short -count=1 ./... (whole module, because of the dependency bump), go test -race -short and go test -race -tags integration -count=1 on internal/{api,config,purchase}, golangci-lint (0 issues) and gocyclo -over 10 on the changed files (silent). All passed.

Body-stale:

• none found

_{Reviewer: rB24. 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 f28e89c1fcc669a176fdeefabfbed27d2ef1a2ae merged onto main (a9b7eb8f), together with #1247, #1248, #1295, #1303, #1310 in that order (combined tree a016458b, 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 f28e89c1 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly f28e89c1fcc669a176fdeefabfbed27d2ef1a2ae.