[IAmJSD]

Root cause

The reshape analysis in cloud-commitments-go can take the region's offered instance types since LeanerCloud/cloud-commitments-go#584 (exchange.WithOfferedInstanceTypes, RunAutoExchangeParams.OfferedInstanceTypes). The platform was pinned to an older pkg and never passed a catalog. All three reshape paths therefore used the legacy size ladder. They could recommend a size the region doesn't sell, or a target in a family the region doesn't offer at all.

Fix

1. Pin bump (d55e3c43): cloud-commitments-go/pkg goes from 541e05e5fd29 to e237996 (the go#584 merge commit), the smallest bump that includes go#584. It is source-compatible. providers/aws stays at aeb8a6f, which already has ec2svc.Client.GetValidResourceTypes.
2. Reshape page (internal/api): reshapeEC2Client gains GetValidResourceTypes. getReshapeRecommendations passes exchange.WithOfferedInstanceTypes(...) to AnalyzeReshapingWithRecs. The new exported api.OfferedInstanceTypes(ctx, region, fetch) wraps the catalog fetch. On an error or an empty result it logs a pkg/logging warning that names the region, then returns nil, so analysis falls back to the legacy ladder. It caches nothing, so every request queries the catalog again and a fallback is never reused.
3. Scheduled + ladder exchanges (internal/server): the standalone task (handler_ri_exchange.go) and the ladder's exchangeRunnerAdapter (ladder_write.go) both set RunAutoExchangeParams.OfferedInstanceTypes through the same helper. Both now build their clients with newRIExchangeClients, so the wiring is shared and tested. The ladder adapter's body after client construction moved to runWithClients so tests can drive it with fakes. Its error messages are unchanged.

IAM: ec2:DescribeInstanceTypeOfferings is already granted in the Lambda/Fargate runtime roles, the CloudFormation stacks and the federation templates, so no IaC change is needed. If a deployment lacks the grant, the call fails, logs the warning and keeps the old behavior.

Closes #1206

Merge order

This PR must merge after #1243 (ri_exchange_mode validation on PUT /api/config). Merged alone, the go#578 fail-closed check below would turn any invalid stored mode into failing scheduled runs, and nothing would stop the API from writing invalid modes again. It also conflicts with #1108 in internal/server/ladder_write.go: whichever merges second must keep #1108's 4-eyes downgrade of unattended exchanges to manual.

Behavior changes to be aware of

• From the pin bump (go#584): reshapes of equal capacity (for example 1x m5.24xlarge to 2x m5.12xlarge) are no longer recommended. Large sizes (32xlarge to 112xlarge) are now analyzed, and metal sizes with no known factor come back as a recommendation with a Reason and no target.
• From the pin bump (go#578), exchange mode: RunAutoExchange now fails closed on any RIExchangeMode other than exactly auto or manual. A stored typo, "" or "Auto" used to auto-execute. It now fails the scheduled run loudly. fix(config): validate ri_exchange_mode on PUT /api/config #1243 (validate ri_exchange_mode on PUT /api/config, closes fix(config): validate RIExchangeMode on PUT /api/config #1196) is the companion fix.
• From the pin bump (go#578), stale-pending cancellation: if CancelPendingExchangesByOrigin fails, RunAutoExchange now returns that error before it starts any exchange, so the scheduled or ladder run fails. Before, the error was only logged and the run went on to create new exchanges while a stale pending one could still be approved. Dry runs are unchanged.
• From the pin bump (go#579), service-details decoding: common.DecodeServiceDetailsFor now returns an error wrapping common.ErrUntypedServiceDetails for a non-empty, non-null Details payload on a service slug with no typed details. It used to return (nil, nil) and drop the payload. memorydb (CacheDetails) and nosql (NoSQLDetails) gained typed codecs so those rows still decode. In the platform, a stored rec on such a slug now fails the purchase (internal/purchase/execution.go) and the detail-identity checks in internal/api/purchase_pricing.go. A malformed request body there is a 400. stampComputeCapacity still ignores decode errors, and recDetailsAvailabilityZone only decodes ec2/compute.
• From the pin bump (go#577), reservation naming: when BuildReservationName has to shorten a name to the 60-character AWS cap (for example OpenSearch r6gd.large.search), it now keeps the random uniqueness suffix. It drops the timestamp first, then the payment segment, then truncates the SKU. Before, it dropped the suffix first, so long names were deterministic and a legitimate repurchase could collide. For RDS, ElastiCache, MemoryDB and OpenSearch this only affects reservation IDs built when no idempotency token is passed. Purchases with a token still use IdempotentReservationID. The EC2 Name tag, a display tag only, is always built this way. The bump changes this for providers/aws too, because Go's minimum version selection resolves its pkg dependency to this pin.
• With a catalog that lacks the RI's family, the reshape page shows a recommendation with Reason: "instance family not offered in region (<family>)..." and no target. Scheduled and ladder runs record it in Skipped with that reason and never look up an offering or request a quote for it.
• Each reshape page load, scheduled run and ladder exchange run makes one extra DescribeInstanceTypeOfferings call (paginated at 1000). I added no caching. The issue requires that the fallback not be cached, and caching successful catalogs could be a follow-up if the latency matters.

Frontend follow-up for the UI owner

No frontend files changed. Two things to check:

• Fewer RIs will show AlternativeTargets, because RIs with no strict downsize are now skipped.
• Recommendations can now arrive with an empty target_instance_type and a reason saying the family isn't offered in the region. The reshape table should show the reason rather than an empty target, or treat these like the existing "completely unused" no-target rows.

Verification

All evidence is unit, mock or fixture based. No real AWS calls were made. Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because this machine's default AWS profile is invalid.

• go build ./..., go vet ./internal/api/ ./internal/server/, and go vet -tags integration on the same two packages: clean.
• go test ./internal/... ./cmd/... -count=1 with only the pin bump applied: all pass.
• go test ./internal/api/ ./internal/server/ ./internal/mocks/ -count=1: pass. go test -short -race ./internal/server/ and -race on the reshape tests in internal/api: pass.
• Integration tests against real Postgres in Docker: go test -tags integration ./internal/api/ -run TestReshapeRecommendations_Integration (4 tests) and ./internal/server/ -run TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient pass.
• New tests:
  • internal/api/reshape_offered_types_test.go:
    • a catalog without m5.2xlarge turns 1x m5.4xlarge at 50% into 2x m5.xlarge instead of 1x m5.2xlarge;
    • a catalog without the family gives an empty target and the Reason;
    • a catalog error or empty catalog gives the legacy target and a warning with the region, and the catalog is queried again on the second request (no cached fallback);
    • unit table for OfferedInstanceTypes, including that a partial result returned next to an error is discarded.
  • internal/server/ri_exchange_offered_types_test.go:
    • newRIExchangeClients routes every call, including GetValidResourceTypes;
    • the same four scenarios run through both the standalone executeRIExchangeReshape and the ladder runWithClients (dry run), asserting the offering lookup target, the outcome target and count, Skipped reasons and the warnings.
• Fail before / pass after:
  • Removing exchange.WithOfferedInstanceTypes(offered) from getReshapeRecommendations fails all 3 handler catalog tests (output shows target m5.2xlarge with no Reason, and no warning).
  • Removing OfferedInstanceTypes: from the standalone params fails the 4 standalone/* subtests.
  • Removing it from the ladder params fails the 4 ladder/* subtests.
• Lint: golangci-lint v2.10.1 run --allow-parallel-runners ./internal/api/... ./internal/server/..., with and without --build-tags integration: 0 issues.
• gocyclo -over 10 -ignore "_test\.go" internal/api internal/server: silent. getReshapeRecommendations stays at 10 because the new call adds no branch.
• scripts/gosec-hook.sh on the changed files: clean. make tidy-check: clean.
• git-secrets: ran in a throwaway repo with an isolated HOME, using --register-aws, scripts/setup-git-secrets.sh and .gitallowed. All 14 changed files were tracked. Planted fake account-ID and AKIA keys were flagged (rc=1), and the clean scan passed (rc=0). scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Not covered: I didn't run against a real AWS account, so a live DescribeInstanceTypeOfferings response and the real IAM grant are untested. I also didn't exercise the production client construction in handleRIExchangeReshape or exchangeRunnerAdapter.RunAutoExchange, since it needs real AWS config. The compiler checks that *ec2svc.Client satisfies riExchangeEC2, and the tests cover newRIExchangeClients itself.

Out of scope

• fix(config): validate RIExchangeMode on PUT /api/config #1196 / fix(config): validate ri_exchange_mode on PUT /api/config #1243 (RIExchangeMode validation), see above.
• Bumping pkg past e237996 (go#587 removes exported API; go#603 and go#609 rename ProviderConfig) and bumping providers/*.
• Caching successful catalogs.

A maintainer, please mirror the issue's triage labels on this PR: priority/p2, severity/low, urgency/this-quarter, impact/few, effort/m, type/feat, triaged.

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

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

[IAmJSD]

Note: the save-time RIExchangeMode check this PR's body refers to (#1196) is now open as #1243. Merging #1243 first (or together) means a bad stored mode is rejected at save time before this bump makes the scheduled run refuse it.

[cristim]

Independent review 2 (claude-fleet, cold): APPROVE 5bcb13b

Merge order note: this PR bumps the pkg pin from 541e05e to e237996, and that range includes go#578.

• go#578's validateMode (pkg@e237996 exchange/auto.go:189-197, called at :232) makes RunAutoExchange fail closed on any mode other than "manual" or "auto".
• fix(config): validate ri_exchange_mode on PUT /api/config #1243 is the PR meant to guarantee that the platform never stores anything else, and it is still REQUEST_CHANGES: PUT /api/config can still store a case-folded or duplicate-key mode.
• So if this lands first, a row holding such a value with exchange enabled would make the scheduled reshape fail after deploy.
• The ladder is unaffected, because its Mode is hard-coded to "auto" (providers/aws@aeb8a6f ladder/reshape.go:133).
• Land fix(config): validate ri_exchange_mode on PUT /api/config #1243's fix first, or check for such rows before deploying this.

Findings (none blocking on their own):

• [non-blocking] internal/api/handler.go:580-590 - The armed() comment says pkg/exchange sends every mode other than "manual" to auto-execute, including "", "Auto" and typos. At the bumped pin, validateMode fails closed instead. The predicate is still safe, because it is conservative, but the comment is stale.
• [non-blocking] go.mod:87 - The pin range also carries more than the body discloses (it names only fix(api): answer 4xx for bad approval tokens and a missing revoke execution #584 and fix(api): GET /api/purchases/cancel/{id} cancels an execution on a side-effecting GET #578):
  • fix(api): stop approving a purchase on GET #577 (the reservation-name suffix);
  • fix(email): RI-exchange approve/reject email links hit POST-only API routes and return 401 #579, chore(ci): type-check integration-tagged tests in make test #580 and fix(auth): write only the admin-editable columns from UpdateUser #581;
  • the e858df6/0dc660d recfilter unit-counting changes;
  • 0dd6bde (cap reshape targets at owned units).
  • The full api and server suites pass, but the body should list these.
• [non-blocking, by design] internal/api/reshape_offered_types.go:18-28 - If the catalog call fails, the code logs a warning and falls back to the legacy ladder.
  • The issue asks for exactly this ("log a warning with the region and proceed without the catalog. Do not cache that fallback").
  • It matches the pkg contract that empty or nil means legacy (pkg exchange/reshape.go:405-418, auto.go:132-134).
  • On the money path, FindConvertibleOffering and the AWS quote remain the backstop. With IAM missing, the degradation is permanent and visible only as a warning.
• [non-blocking, test gap] Probe M9, the ladder AccountID: clients.accountID replaced with "", survives the new tests. The constructor test does assert c.accountID.
• [note] The catalog is region-level (LocationTypeRegion). A zonal RI's target can be offered in the region but not in its AZ, so that case is still left to the offering lookup or the quote. This is unchanged from before.

Attacked:

• Source, read from the module cache, with the h1 sum matching go.sum: GetValidResourceTypes in providers/aws@aeb8a6f services/ec2/client.go:692-725.
  • It loops over NextToken with MaxResults 1000, and it is scoped to the client's configured region.
  • Any page error returns nil and the error, never a partial list.
  • The helper also discards a partial result returned with an error (M4).
• Analysis, in pkg@e237996 exchange/reshape.go:410-457 and 564-577:
  • Targets come only from the per-family offered ladder.
  • A family missing from the catalog gets a Reason and no target, and RunAutoExchange skips it before any offering lookup (auto.go:286-296).
  • Type names are split with SplitN on "." and not otherwise normalised. AWS enums and RI types are both lowercase.
• Callers: the only reshape and auto-exchange entry points are the API reshape page (handler_ri_exchange.go:1760), the standalone scheduled run (server/handler_ri_exchange.go:178) and the ladder adapter (ladder_write.go:79).
  • ReshapeBuffer (providers/aws ladder/reshape.go:97) reaches the ladder adapter.
  • Region and account are consistent: the API uses the same cfg and ec2Client as the RI listing, and the ladder uses a.region for config, utilisation and the catalog.
• Mutation probes on a scratch copy, each restored and confirmed identical with cmp:
  • M1, the API catalog set to nil: caught.
  • M2, the standalone run drops the catalog: 4 subtests FAIL.
  • M3, the ladder drops the catalog: 4 subtests FAIL.
  • M4, the helper returns a partial result on error: caught.
  • M5, the constructor stubs out the catalog: caught.
  • M6 and M7, the API and ladder warning region blanked: caught.
  • M8, the empty-catalog warning removed: caught.
  • M9, the ladder AccountID blanked: survives. This is the test gap above.
• Fail-before: with the head's go.mod and tests on base, both test packages fail to compile. M1-M3 are the semantic fail-before for each path.

Verified locally, with GOTOOLCHAIN=go1.26.10 and no cloud credentials, under the heavy lock:

• go build ./... and go vet ./internal/api/ ./internal/server/, plain and with -tags integration: ok. gofmt -l on the 12 changed files: clean.
• go test -count=1 ./internal/api/ ./internal/server/: ok. -race on the new tests: ok.
• git merge-tree --write-tree origin/main 5bcb13b against origin/main 6af022f: clean.
• No migration, frontend or terraform changes.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

[cristim]

Merge-order note: this pin bump (541e05e..e237996) includes LeanerCloud/cloud-commitments-go#578, which makes the RI-exchange mode check fail closed. It should merge only after #1243 (ri_exchange_mode validation on PUT /api/config) is fixed and merged, and after the legacy-row query in #1243's body shows no stored mode other than auto/manual. Otherwise a stored Auto or automatic mode with exchange enabled would stop the scheduled reshape. The range also brings in go #577, #579-#581, the recfilter unit-counting change and 0dd6bde (cap reshape targets at owned units); worth listing in the body.

[cristim]

VERDICT: APPROVE 5bcb13b

PR #1246 (draft, @IAmJSD), head 5bcb13b, merge base 503eb2e (3 behind origin/main 2ded648).
Merge-tree vs origin/main: clean (tree 8ce8bdd3). Also clean vs #1243 (9616ae7) and vs #1240. Merged tree builds; vet (plain + integration) clean; go mod tidy -diff clean; go mod verify OK.
Merge gate: must land after #1243 (still an open draft). go#578 makes RunAutoExchange fail closed on any mode other than auto/manual.

Findings (no blockers)

• low (follow-up) frontend/src/riexchange.ts:985,990 - no-target recs (unoffered family, and metal with no factor, now emitted by go#584/0dd6bde) render as "0x " with an active Exchange button. Idle RIs already had this. The PR flags it for the UI owner, but no issue is filed. File one.
• nit internal/api/handler.go:586-600 - the armed() comment says pkg routes every non-"manual" mode to auto; after this bump it fails closed. The predicate is still safe (stricter than needed). fix(config): validate ri_exchange_mode on PUT /api/config #1243 rewrites this block; whichever lands second should fix the wording.
• nit internal/server/handler_ri_exchange.go:19 / ladder_write.go:14 - server imports the exported api.OfferedInstanceTypes. server already imports api (app.go), so there is no new edge. Fine as is.
• Largest commit: e7754dd at 392 changed lines, under the 400 cap.

#1206 checklist vs code

• Pin bump: go.mod pkg 541e05e5fd29 to e237996e45b4. providers stay at aeb8a6f/3def3f8.
• GetValidResourceTypes reaches all three call sites: api/handler_ri_exchange.go:1760 (reshapeEC2Client:50), server/handler_ri_exchange.go:178 (standalone), ladder_write.go:79 (ladder). Region is consistent: the ladder's awsCfg uses WithRegion(a.region); the page uses p.region.
• Fallback: reshape_offered_types.go:18-28 warns with the region on an error or an empty catalog, discards a partial result, and caches nothing.
• Tests cover catalog present, missing family (Reason), error/empty with no caching, for the page, standalone and ladder.

Go-lib range (pkg-only source diff from the module cache)

• fix(api): GET /api/purchases/cancel/{id} cancels an execution on a side-effecting GET #578 validateMode / cancelStalePending: the ladder passes Mode=auto (providers/aws ladder/reshape.go:133). The standalone mode comes from the DB, which defaults to 'manual' and coerces "" to manual (store_postgres.go:313). A cancel failure now aborts the run instead of warning. This is fail-loud and acceptable.
• fix(email): RI-exchange approve/reject email links hit POST-only API routes and return 401 #579 DecodeServiceDetailsFor rejects a payload for an untyped slug. Platform callers: purchase/execution.go:1162, purchase_pricing.go:200-232, handler_purchases.go:3415 (EC2 only), stampComputeCapacity (error-tolerant). The untyped slugs are storage/commitments/other. No pinned provider stamps Details for them (GCP cloudstorage sets none). MemoryDB and nosql now decode to typed structs, so they get engine fallback only. No break.
• fix(api): stop approving a purchase on GET #577 reservation names keep the random suffix (idempotency improves). fix(auth): write only the admin-editable columns from UpdateUser #581 logging SetOutput is race-free. chore(ci): type-check integration-tagged tests in make test #580 retry clamps jitter. The scorer/recfilter/reporter/DropSummary changes are not imported by the platform except through providers.
• 0dd6bde/fix(api): answer 4xx for bad approval tokens and a missing revoke execution #584 cap targets at owned units, skip equal-capacity reshapes, metal returns a Reason. These are covered by updated platform tests.

Attacked / probes (all killed)

• Remove WithOfferedInstanceTypes from the page: CatalogLimitsTargetToOfferedSizes and CatalogMissingFamilySurfacesReason fail.
• Drop OfferedInstanceTypes from the ladder params: TestRIExchangeReshape_OfferedCatalog fails.
• Drop it from the standalone params: the same test fails.
• Return partial types on a fetch error: TestOfferedInstanceTypes fails.
  Results on the merged tree: unit tests for api, server, purchase, scheduler, mocks, database pass (6004). Integration -run "Reshape|RIExchange" for api and server: PASS.
  CI on head: all green (Snyk skipped). It ran on the old base 503eb2e; the merged tree passes locally.

[IAmJSD]

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

Blocking:

• internal/server/ri_exchange_offered_types_test.go:231: the table test builds its clients with account ID "acct-1". Main gained a 12-digit AWS account-ID check in the exchange store adapter today (fix(db): let ri_exchange_history.account_id hold Azure and GCP identifiers #1170, f56cecb; main internal/server/handler_ri_exchange.go: "refusing to record RI exchange: account ID %q is not a 12-digit AWS account ID"). The PR's Unit Tests ran at 12:29Z, before that landed. After merging current origin/main into this head locally (merges cleanly), TestRIExchangeReshape_OfferedCatalog/standalone/{catalog_limits_the_target_to_offered_sizes, catalog_error_falls_back_to_the_legacy_ladder, empty_catalog_falls_back_to_the_legacy_ladder} fail with "[]" should have 1 item(s). The standalone run is not a dry run, so its pending-record save is refused; the ladder subtests are dry runs and pass. Merging as-is turns main's Unit Tests red. Fix: use a 12-digit ID such as "123456789012" at :121/:142/:231, merge origin/main, and re-run CI.

Non-blocking:

• The pin bump 541e05e5 -> e237996e brings more than go#584 and go#578, and the body doesn't list the rest:

  • common.DecodeServiceDetailsFor now errors on a non-empty payload for an untyped or unknown service slug. It used to return (nil, nil). This reaches the purchase path (internal/purchase/execution.go decode) and recDetailsAvailabilityZone.
  • MemoryDB and NoSQL rows now decode into typed details.
  • RunAutoExchange now fails the run when CancelPendingExchangesByOrigin errors. It used to log a warning and continue.
  • Reservation names keep their random suffix (it is used as the purchase idempotency name).
  • Assorted fixes in recfilter dedupe, scorer validation and retry.

  I checked the providers pinned on main. Every service slug they stamp is typed in the new codec, and GCP storage recs carry no Details, so I found no live break. The full go test -short ./internal/... ./cmd/... passes. Please list these in the body so the deploy note is complete.

• The go#578 mode fail-closed: as the body says, fix(config): validate RIExchangeMode on PUT /api/config #1196 should land first or together with this PR, or stored invalid modes should be checked before deploy.

• Conflicts textually with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 in internal/server/ladder_write.go. Both PRs split the ladder adapter (run vs runWithClients), so whichever merges second has to reconcile by hand. The 4-eyes mode downgrade from fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 must survive in the combined function.

• internal/server now calls api.OfferedInstanceTypes. That works (server already imports api in app.go/http.go), but the helper isn't HTTP-specific. If more server code needs it, internal/exchange or similar would be a better home. Cosmetic.

Attacked:

• Diffed the two pinned pkg module trees file by file (exchange, common, recfilter, scorer, retry, logging, config) to see what the bump brings. Grepped platform callers of the changed APIs and the service slugs the pinned aws/azure/gcp providers emit.
• Fail-before:
  • Dropping exchange.WithOfferedInstanceTypes(offered) fails TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes and TestGetReshapeRecommendations_CatalogMissingFamilySurfacesReason.
  • Dropping the OfferedInstanceTypes param (keeping the fetch) in both server paths fails the 4 catalog-dependent TestRIExchangeReshape_OfferedCatalog subtests.
  • Restored afterwards.
• Ran on the head: go build ./...; go vet with and without -tags integration on api/server; go test -race -short -count=1 ./internal/api/ ./internal/server/ ./internal/mocks/ (ok); go test -short -count=1 ./internal/... ./cmd/... (ok).
• Ran on the head merged with origin/main: build ok, api ok, server FAIL as above; golangci-lint with and without --build-tags integration 0 issues; gocyclo silent.

Body-stale:

• "smallest bump that includes go#584" plus the behavior-change section: the bump also changes service-details decoding, stale-pending cancellation and reservation naming (see above), and the body doesn't mention them.

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

Updated the branch. New head: 46cce856 (46cce856cceaebabd066949ba9c46fb5c6fe4e4f).

• Merged origin/main twice with merge commits, first at 7cde88e and then at 26004a6 (fix(purchase): guard the remaining post-CAS full-row saves on status #1111, test(purchase): expect #1245's failure alert in the post-CAS safeFail race test (fixes red main) #1418). There were no conflicts.
• Fixed the fixture that broke under main's 12-digit account ID validation (fix(db): let ri_exchange_history.account_id hold Azure and GCP identifiers #1170). "acct-1" in internal/server/ri_exchange_offered_types_test.go made three TestRIExchangeReshape_OfferedCatalog/standalone/* subtests fail: the exchange record was refused before any assertion ran. It now uses the 123456789012 placeholder. Every assertion is kept, including assert.Equal(t, "123456789012", c.accountID).
• ladder_write.go was not touched.
• Updated the PR body:
  • It now lists three more behavior changes from the pkg pin bump: go#579 service-details decoding, go#578 stale-pending cancellation failing the run, and go#577 reservation naming.
  • It now says this PR must merge after fix(config): validate ri_exchange_mode on PUT /api/config #1243 (ri_exchange_mode validation).
  • It now notes the ladder_write.go conflict with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108, and that whichever merges second must keep fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108's 4-eyes downgrade.

Checks on the merged tree:

• go build, go vet (plain, integration, e2e): clean.
• golangci-lint v2.10.1 on internal/server and internal/api, with and without --build-tags integration: 0 issues. gocyclo: silent. go mod tidy -diff: clean.
• go test -race -short on ./internal/server/ and ./internal/api/: ok.
• go test -race -tags integration on ./internal/server/ and ./internal/api/: ok on the first merge (7cde88e). The machine was heavily loaded, and TestIntegration_HistorySummary_TotalsCoverAllInScopeRows hit its 3-minute budget while the shared Postgres template was being built. It passed when rerun alone. I did not rerun integration after the second merge, which only brought in purchase/api changes from fix(purchase): guard the remaining post-CAS full-row saves on status #1111 and test(purchase): expect #1245's failure alert in the post-CAS safeFail race test (fixes red main) #1418.
• Secrets scan: clean, and the planted key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS.

[IAmJSD]

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

Blocking:

• none in the change itself. Merge-order and merge-state items below must be handled before this lands.

Non-blocking:

• Merge state (security-relevant): this head CONFLICTS with main in internal/server/handler_ri_exchange.go and internal/server/ladder_write.go, and in internal/api/handler_purchases.go/its test. On main, fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 (bdfe866) wraps the standalone mode as fourEyesExchangeMode(cfg, cfg.RIExchangeMode), and fix(server): stop reporting fabricated success on three operator paths #1264 (f05b0e4) made resolveAccountID return an error and abort, replacing the old "unknown". This PR rewrites both call sites through newRIExchangeClients(..., resolveAccountID(ctx, awsCfg)). A careless resolution could drop the 4-eyes downgrade of unattended exchanges, or reintroduce the old resolveAccountID signature. The merge commit needs a fresh review that checks both are kept, in the standalone path and in the ladder adapter.
• Merge order: the body says this must merge after fix(config): validate ri_exchange_mode on PUT /api/config #1243, which is still OPEN. The pinned go#578 makes RunAutoExchange fail closed on any mode other than exactly auto/manual. If this lands first, a stored case-variant mode makes the scheduled reshape fail. That fails loud, not open, so I'm not blocking on it, but land fix(config): validate ri_exchange_mode on PUT /api/config #1243 first or check ri_exchange_mode values before deploying.
• go.mod: this PR bumps pkg to e237996, and fix(onboarding): flag aws-cn accounts and refuse their Savings Plans purchases #1248 bumps providers/aws (which moves pkg to ff6225d). The two will conflict on go.mod/go.sum. With MVS, e237996 has the higher pseudo-version, so whichever lands second has to re-tidy and re-test.
• go#579 (DecodeServiceDetailsFor now rejects a Details payload on an untyped slug): I checked the pinned providers. AWS, Azure and GCP only stamp Details on typed slugs (compute, relational-db, cache, memorydb, nosql, search, data-warehouse and the Savings Plans slugs). GCP storage sets none. I see no break today, but it is a fail-closed change on the purchase path to keep in mind.
• internal/api/handler.go armed() comment: it still says pkg auto-executes any mode other than "manual". After this bump it fails closed. The predicate is still safe; the comment is stale.
• Frontend: recommendations with no target (family not offered in the region) will render as an empty target. The body flags this for the UI owner, but no issue has been filed yet.

Attacked:

• The catalog fetch reaches all three entry points: the API reshape page, the standalone scheduled run and the ladder adapter. Region is consistent (p.region, awsCfg.Region, a.region). On an error or an empty result it falls back to the legacy ladder with a warning, discards a partial result and caches nothing. newRIExchangeClients keeps the exchange client, offering lookup and utilization region wiring the same as before.
• Fail-before: I removed exchange.WithOfferedInstanceTypes(offered) from getReshapeRecommendations and the OfferedInstanceTypes: params from the standalone and ladder runs. TestGetReshapeRecommendations_CatalogLimitsTargetToOfferedSizes, ..._CatalogMissingFamilySurfacesReason and TestRIExchangeReshape_OfferedCatalog (standalone and ladder subtests) failed. They passed after restoring.
• Ran go build ./..., go vet ./..., go vet -tags integration ./..., go test -short -count=1 ./... (whole module, because of the pin bump: all ok), go test -race -short -count=1 on internal/{api,server} (ok), golangci-lint with and without the integration tag (0 issues), gocyclo -over 10 (silent), and go test -race -tags integration -count=1 on internal/{server,api} (ok).

Body-stale:

• "It also conflicts with fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 in internal/server/ladder_write.go": fix(ri-exchange): run unattended exchanges as manual while 4-eyes is on #1108 has merged (2026-10-25), so this is now a conflict with main, together with fix(server): stop reporting fabricated success on three operator paths #1264 and refactor(ri-exchange): share the convertible RI monthly cost formula #1260/fix(exchange): show unknown-outcome auto-exchanges as needing reconciliation in the notification email #1278 in internal/server/handler_ri_exchange.go.

From earlier reviews (confirmed): #1243 is still in REQUEST_CHANGES, so "merge after #1243" is not yet satisfiable. The stale armed() comment was also raised there.

_{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.}