[IAmJSD]

Refs #613

Why

Settings writes Azure Cosmos DB service configs and per-account overrides under nosql, the slug Cosmos recommendations carry (common.ServiceNoSQL). The backend slug table, mapServiceSlug in internal/purchase/execution.go, did not list it. So settings-service-ids.test.ts, which checks every Settings id against that table, had to carry azure/nosql as a named exception.

What

• internal/purchase/execution.go: adds "nosql": common.ServiceNoSQL to the canonical slugs in mapServiceSlug and updates its doc comment. Runtime dispatch doesn't change. mapServiceType("nosql") already returned common.ServiceNoSQL through its verbatim fallback, and the pinned Azure provider's GetServiceClient already handles common.ServiceNoSQL. The AWS no-token re-drive guard still doesn't cover nosql (it only matches EC2/compute/Redshift/data-warehouse), and the Azure reservation re-drive stays allowed.
• internal/purchase/slug_alias_drift_test.go: new TestMapServiceSlugAcceptsEveryCanonicalReservationSlug, which pins compute, relational-db, nosql, cache, search and data-warehouse.
• internal/purchase/coverage_extra_test.go and redrive_guard_reach_test.go: add nosql to the lists that enumerate mapServiceSlug's keys.
• frontend/src/__tests__/settings-service-ids.test.ts: removes the NOT_IN_PURCHASE_SLUGS = ['azure/nosql'] exception. Every Azure/GCP id the Settings cards save and the override list offers must now be a backend slug, with no exceptions.

What remains on #613

• Legacy account_service_overrides rows saved under the old ids (azure/vm, azure/sql, azure/cosmosdb, azure/redis, gcp/sql, gcp/memorystore) are still only flagged in the UI. They need a manual re-create or a backend SQL migration, and old service_configs rows under those ids are never deleted. This PR does not add that migration on purpose: the migration numbers are held by a chain of open PRs, so it should come as a separate change at 000149 or later.

Verification

• Fail before (Go): with main's execution.go, TestMapServiceSlugAcceptsEveryCanonicalReservationSlug fails with mapServiceSlug rejects canonical slug "nosql".
• Fail before (frontend): with main's execution.go and the exception removed, settings-service-ids.test.ts fails, receiving ["azure/nosql"] where it expects []. Both pass after.
• go build ./... && go vet ./... && go vet -tags integration ./... && (cd tests/e2e && go vet -tags e2e ./...): pass.
• golangci-lint v2.10.1 on ./internal/purchase/..., with and without --build-tags integration: 0 issues. gocyclo -over 10 internal/purchase/execution.go: nothing reported.
• go test -race -short -count=1 ./internal/purchase/: ok.
• go test -race -tags integration -count=1 ./internal/purchase/: 10 tests failed on the first full run when the shared Postgres container was lost on a heavily loaded host. All 10 passed on rerun (TestApproveAzurePricingRequiresMigrationAndRecollection, TestCancelExecution_ScheduledRowIsRefusedWithRevokeHint, TestFourEyes_Sweep*, TestReconcileLadderTranches_*, TestManualPlanStep_*).
• Frontend (from frontend/, after npm ci): npm run -s lint && npm run -s typecheck && npm run -s build && npx jest --no-coverage --silent all pass (171 suites, 4501 tests passed, 1 skipped).
• Playwright, full suite via the local private-port runner: 267 passed.
• Secrets scan: clean (planted key flagged). scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

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: ecf8493
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7d46ffb9-515e-4f1d-b95d-e0b06caa9f17

Commits

Reviewing files that changed from the base of the PR and between ecf8493 and 0ae44ab.

Files selected for processing: 5

• frontend/src/__tests__/settings-service-ids.test.ts
• internal/purchase/coverage_extra_test.go
• internal/purchase/execution.go
• internal/purchase/redrive_guard_reach_test.go
• internal/purchase/slug_alias_drift_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]

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

Blocking:

• none

Non-blocking:

• internal/purchase/execution.go:1244-1248: the mapServiceType doc comment still lists the canonical slugs without nosql. The mapServiceSlug comment was updated, so the two now disagree.

Attacked:

• Money-path reach: mapServiceSlug has two callers, mapServiceType (dispatch) and awsPurchaseHasNoDuplicateGuard (re-drive refusal). For "nosql", mapServiceType returned ServiceType("nosql") before through its verbatim fallback, and common.ServiceNoSQL == "nosql" in the pinned pkg, so dispatch is byte-identical. Main's execution.go with the PR's TestMapServiceType_AllBranches (which now includes nosql) still passes, which proves this. awsPurchaseHasNoDuplicateGuard("nosql") returned false before (!ok) and still returns false (not EC2/compute/Redshift/data-warehouse). The Azure re-drive gate only refuses savings plans. So no purchase, re-drive or refusal decision changes, and the pinned Azure GetServiceClient already handles ServiceNoSQL.
• Fail-before (Go): with main's execution.go, TestMapServiceSlugAcceptsEveryCanonicalReservationSlug fails with mapServiceSlug rejects canonical slug "nosql". Fail-before (frontend): with main's execution.go, settings-service-ids.test.ts fails (1 of 11). Both pass with the PR.
• Ran: go build ./..., go vet on internal/purchase with and without -tags integration, go test -race -short -count=1 ./internal/purchase/ (ok), go test -race -tags integration -count=1 ./internal/purchase/ (ok, 128s), npx jest settings-service-ids.test.ts (11 passed). Refs fix(frontend): settings service overrides still use the old Azure/GCP service ids #613 is right: the legacy-id rows still need a migration.

Body-stale:

• none

_{Reviewer: rE2. 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 df15513bdb223b852db45b0905da4ae38b58c344

Blocking:

• none

Non-blocking:

• The PR currently conflicts with main (GitHub: CONFLICTING). git merge-tree origin/main HEAD shows one conflict, in internal/purchase/slug_alias_drift_test.go: main's chore(config): tie suppression slug aliases to the library service constants #1425 and fix(config): canonicalise dash-free Savings Plans spellings in suppressions #1456 changed that file. Since then, main has also bumped cloud-commitments-go (chore(deps): bump cloud-commitments-go to 9cec521 #1466). The merge needs a merge commit, and the new head needs a fresh review.
• internal/purchase/execution.go, the mapServiceType doc comment (just above func (m *Manager) mapServiceType), still lists the canonical slugs as "compute, relational-db, cache, search, data-warehouse". The mapServiceSlug comment was updated to include nosql; this one was not.

Attacked: I checked that this is a runtime no-op. common.ServiceNoSQL is "nosql" in the pinned pkg, so mapServiceType("nosql") returns the same value it used to get through the verbatim fallback. The only other caller, awsPurchaseHasNoDuplicateGuard, still returns false for nosql (it used to return false through !ok; now the switch has no ServiceNoSQL case). The re-drive gating for Azure Cosmos is therefore unchanged, and there is no new purchase path. Fail-before: with the merge-base execution.go, TestMapServiceSlugAcceptsEveryCanonicalReservationSlug fails ("rejects canonical slug "nosql"") and the jest settings-service-ids.test.ts fails, receiving ["azure/nosql"]. Both pass on the head. Ran go build ./..., go vet (plain and integration) on internal/purchase, go test -race -short -count=1 ./internal/purchase/ (ok), go test -race -tags integration -count=1 ./internal/purchase/ (ok, 77s), and npm ci + npx jest src/__tests__/settings-service-ids.test.ts (11 passed). Refs #613 is right, since the legacy override migration remains.

Body-stale:

• none

_{Reviewer: rE5. 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 0ae44abef4cfb5ae2fcc5ba78a8b4edf57e535ca

Blocking:

• none

Non-blocking:

• internal/purchase/execution.go:1249 (mapServiceType doc comment): it still lists the canonical slugs as "compute, relational-db, cache, search, data-warehouse". Add nosql there too, so the two comments agree.

Attacked:

• Money path: I checked that this changes no runtime dispatch.
  • common.ServiceNoSQL is the literal "nosql" (cloud-commitments-go pkg/common/types.go:88). Before this PR, mapServiceType("nosql") already returned common.ServiceType("nosql") through its verbatim fallback, so the purchase client gets the same value.
  • mapServiceSlug has two callers. One is mapServiceType, used by execution, partition_guard's Savings Plans check and suppression canonicalisation. The other is awsPurchaseHasNoDuplicateGuard, which previously returned false on !ok and now returns false through the default branch. So the AWS re-drive refusal and the Azure re-drive decision are unchanged.
• Ran:
  • go build ./...; go vet (with and without -tags integration) on ./internal/purchase/.
  • go test -race -short -count=1 ./internal/purchase/ (pass).
  • go test -race -tags integration -count=1 ./internal/purchase/ (pass, 80s).
  • Frontend: npm ci, then jest src/__tests__/settings-service-ids.test.ts (11 pass).
• Fail-before, with internal/purchase/execution.go checked out from origin/main:
  • TestMapServiceSlugAcceptsEveryCanonicalReservationSlug fails with mapServiceSlug rejects canonical slug "nosql".
  • settings-service-ids.test.ts fails (1 of 11) on the now-empty exception list.
  • Both restored afterwards.
• Refs #613 (not Closes) is right: the legacy-id override rows still need the migration the body describes.

Body-stale:

• none

_{Reviewer: rE7. 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 0ae44abef4cfb5ae2fcc5ba78a8b4edf57e535ca

Blocking:

• none

Non-blocking:

• internal/purchase/execution.go doc comment: "nosql is Azure Cosmos DB". The library constant common.ServiceNoSQL also covers DynamoDB and Firestore (types.go comment in cloud-commitments-go). It would be more accurate to say Cosmos DB is the provider that currently uses it.
• The required CI Success check is green. The red Playwright Chromium check on this head is a runner flake (Install Chromium timed out during apt install) and is not related to the diff.
• From an earlier review, and still true at this head: the mapServiceType doc comment (execution.go:1244-1248) still lists the canonical slugs without nosql, so it now disagrees with the mapServiceSlug comment.

Attacked:

• Sensitive-path check: I traced every caller of mapServiceSlug to see whether adding nosql opens a purchase or re-drive path that was refused before.
  • mapServiceType returns common.ServiceType("nosql") before and after, because ServiceNoSQL == "nosql" and the verbatim fallback already produced it. So dispatch is unchanged.
  • awsPurchaseHasNoDuplicateGuard returns false before (!ok) and after (NoSQL is not in its switch), so the re-drive guard is unchanged.
  • The AWS provider at the pinned version does not reference ServiceNoSQL. Only the Azure provider and its cosmosdb client do.
  • The config-side awsLegacyServiceSlugs check is unaffected.
  • Net runtime change: none. The PR is a table and test consistency fix, as claimed. Refs #613 (not Closes) is right, because legacy override rows remain.
• Fail-before: with main's execution.go, TestMapServiceSlugAcceptsEveryCanonicalReservationSlug fails ("mapServiceSlug rejects canonical slug "nosql""). It passes with the fix.
• Ran: go build ./..., go vet ./... and go vet -tags integration ./... (clean). go test -race -short -count=1 ./internal/purchase/ ./internal/config/ (ok). go test -race -tags integration -count=1 ./internal/purchase/ (ok, 75s). I did not run the frontend jest locally. CI's Build frontend (the pre-merge jest gate) passed on this head.

Body-stale:

• "it should come as a separate change at 000149 or later": per the current migration plan, the next free number is 000146 or above. That is not wrong, but the number is stale.

_{Reviewer: rE8. 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 0ae44abef4cfb5ae2fcc5ba78a8b4edf57e535ca merged onto main (df0de7e7), together with #1289, #1459 in that order (combined tree 2f911bc2, 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 0ae44abe (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 0ae44abef4cfb5ae2fcc5ba78a8b4edf57e535ca.