Skip to content

fix(azure): roll out reservation fixes and preserve exchange scopes - #475

Merged
cristim merged 5 commits into
mainfrom
codex/go76-platform-azure-rollout
Oct 8, 2026
Merged

cristim merged 5 commits into
mainfrom
codex/go76-platform-azure-rollout

Conversation

@cristim

@cristim cristim commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

This rolls the Azure reservation fixes into the platform and preserves server-owned reservation scope through exchange preview and execution. The upgraded provider rejects missing scope metadata; both handlers now reuse the owned reservation listing and replace only the requested quantity.

Azure is pinned to v0.0.0-20261007130138-e24345ce68f8, with shared pkg v0.0.0-20261006104817-90e61e668b99. AWS and GCP pins are unchanged. Five quote fixtures now supply the required USD currency.

Verification uses realistic synthetic Azure SDK/HTTP responses through the actual authenticated handlers, concrete Azure client and purchase manager. Four Shared/Single preview/execution success cases and ten metadata/billing refusal cases pass; all four success cases fail with the original handler. An independent reviewer reproduced this evidence. Monthly/upfront purchases and retries pass through the concrete purchase manager; the old Azure pin reproduces incorrect recorded costs in all four control cases.

No live-cloud purchase or exchange was performed. Live account/catalog acceptance remains an honestly reported coverage gap, not a merge gate under the owner's current instructions. Final exact-head review, fresh build/full race-short suite/vet and CI evidence will be recorded in the merge evidence comment.

Closes #650
Refs LeanerCloud/cloud-commitments-go#76 and LeanerCloud/cloud-commitments-go#132

Summary by CodeRabbit

  • Bug Fixes
    • Azure reservation exchange previews and executions now preserve reservation details, billing scope, applied scopes, and requested quantities.
    • Requests with missing or unsupported scopes, incompatible scope combinations, or billing scopes outside the tenant are rejected before exchange processing.
    • These updates help ensure exchange calculations and executions use the same validated reservation information.

@cristim cristim added urgency/this-sprint Within the current sprint triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm impact/many Affects most users effort/m Days type/bug Defect labels Oct 4, 2026
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 90f908c5-7377-4c74-8026-43ccc8fde732
📥 Commits

Reviewing files that changed from the base of the PR and between 0b3480c and 5009085.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • internal/api/handler_ri_exchange.go
  • internal/api/handler_ri_exchange_azure_scope_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

Azure exchange preview and execution now use tenant-listed reservation metadata with requested quantities. The Azure provider dependency changed. Exchange quote fixtures now specify "USD". Tests cover preserved scope data and rejected requests.

Changes

Azure exchange updates

Layer / File(s) Summary
Update module requirements and quote fixtures
go.mod, internal/server/handler_ri_exchange_test.go
The requirements for cloud-commitments-go/pkg and cloud-commitments-go/providers/azure changed. Manual-mode, auto-mode, and daily-cap quote and execution fixtures now set CurrencyCode to "USD".
Use verified reservations for preview pricing
internal/api/handler_ri_exchange.go, internal/api/handler_ri_exchange_azure_scope_test.go
The ownership check returns tenant-listed reservations with requested quantities. The compatible-offerings path passes those sources to CalculateExchange. Tests cover preserved scope data and rejected requests.
Reuse verified reservations for execution
internal/api/handler_ri_exchange.go, internal/api/handler_ri_exchange_azure_scope_test.go
Execution authorization returns the verified reservation sources with the client. The execute handler uses those sources for its re-quote. Tests cover execution scope data and session IDs.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ExchangeHandler
  participant AzureClient
  participant CalculateExchange
  ExchangeHandler->>AzureClient: List tenant reservations
  ExchangeHandler->>ExchangeHandler: Check ownership and apply requested quantities
  ExchangeHandler->>CalculateExchange: Price verified reservation sources
Loading
sequenceDiagram
  participant ExecuteHandler
  participant AzureClient
  participant CalculateExchange
  ExecuteHandler->>AzureClient: List and verify tenant reservations
  ExecuteHandler->>CalculateExchange: Re-quote verified sources
  ExecuteHandler->>AzureClient: Execute exchange using authorized client
Loading

Merge Risk: ⚪ Minimal · up to 50090

Azure exchange preview and execution now keep server-owned reservation scopes and change only the requested quantity. No concrete merge-blocking issue was found. Live-cloud acceptance has not been tested, so the final CI results should be confirmed before merging.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Issue #650 is limited to the Azure dependency pin and related dependency compatibility. The changes to internal/api/handler_ri_exchange.go and the new Azure exchange scope tests implement reservatio… Remove the reservation-scope production changes and their dedicated tests from this issue's pull request, or associate that work with a directly linked issue that defines the requirement.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Azure reservation fixes and the preservation of exchange scopes, which are the main changes in the pull request.
Linked Issues check ✅ Passed Issue #650 requires providers/azure at or after e24345c. go.mod pins providers/azure to v0.0.0-20261007130138-e24345ce68f8. AWS and GCP pins remain unchanged. The pkg pin uses a different …
Full details: Out of Scope Changes check

Explanation

Issue #650 is limited to the Azure dependency pin and related dependency compatibility. The changes to internal/api/handler_ri_exchange.go and the new Azure exchange scope tests implement reservation-scope preservation and refusal behavior. The linked issue does not request this production behavior, and the referenced pull requests do not establish linked-issue scope.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cristim

cristim commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

CI result for exact commit 369728d5ecaa45cc9191bcd8d59c8f35b2757056:

  • pre-commit run 37225100804 passed.
  • CI - Build & Test run 37225100807 failed. Unit Tests, Integration Tests, E2E Tests, lint, Terraform validation, and the other successful jobs passed. The failing steps were Scan the shipped image for Go advisories in Build Docker Image, plus Run govulncheck CVE scanner (all modules) and Run npm audit (frontend) in Security Scanning. The CI Success aggregate failed as a result.
  • Both Go scans flagged fixable GO-2026-6505 in go.opentelemetry.io/otel/sdk@v1.44.0 (fixed in v1.45.0). npm audit reported 34 frontend vulnerabilities, 32 high. The parent commit and this commit both pin otel/sdk@v1.44.0; this PR changes only go.mod and go.sum for Azure and required pkg versions, and leaves the frontend unchanged. These findings belong to the existing security baseline tracked by PR #473.

Keep this PR open. Do not treat the passing affected-path tests as green CI or full security coverage. Verify the combined tree and required CI after #473 lands; the upstream Azure stack and real-scenario acceptance also remain open.

@cristim
cristim force-pushed the codex/go76-platform-azure-rollout branch from db21110 to 4345f7e Compare October 7, 2026 12:35
@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Rebased onto current main (3131779) to clear the DIRTY merge state.

Conflict shape: go.mod/go.sum only - main had bumped providers/aws to v0.0.0-20261003204812-9962786e0695 while this PR bumps providers/azure to v0.0.0-20261004143708-56555e1be095. Resolution keeps both bumps (newest of each); go.sum regenerated via go mod tidy.

Verified locally: go build ./... and go test ./internal/... ./cmd/... all pass.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

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

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Coordination note from the w2 shard (orchestrator rollout relay for cloud-commitments-go#218): upstream PR #218 merged to main as e24345c — fix(azure): stop dividing the monthly run-rate by the term months. This PR (#475) pins providers/azure at 56555e1be095 (2026-10-04), which predates that merge. I opened #652 as the focused rollout of the #218 fix: azure pinned at exactly e24345ce68f8, pkg bumped to the azure module's required 90e61e668b99, aws/gcp untouched. Both PRs touch go.mod/go.sum azure pins — whichever merges second should rebase and keep the newer pin (or, if you prefer to absorb the #218 pin into this PR, say so and I'll close #652).

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Round summary for head 11fdc1fd:

  • The branch was rebased onto current main (3131779c) earlier today as 4345f7e; CodeRabbit reviewed the rebased changeset (369728d..4345f7e) and reported "No actionable comments were generated in the recent review." There are no outstanding findings to bucket: nothing fixed, nothing stale, nothing dismissed.
  • New commit 11fdc1fd chore(deps): pin providers/azure at e24345c for the monthly run-rate fix. The previous pin 56555e1be095 had diverged from cloud-commitments-go main (9 ahead / 62 behind) and did not contain the merged monthly run-rate fix (e24345ce68f8, cloud-commitments-go#218). The pin now resolves v0.0.0-20261007130138-e24345ce68f8; the shared pkg pin advanced to v0.0.0-20261006104817-90e61e668b99 only because the new Azure module requires it. providers/aws and providers/gcp are unchanged. This closes chore(deps): pin providers/azure at or after cloud-commitments-go e24345c (Azure monthly run-rate fix) #650.
  • Consumer check against the delta's breaking/units changes: common.PurchaseResult gained a field (platform uses keyed literals only, compile clean), and the Azure Consumption units change (CommitmentCost = 0 for monthly variants, true monthly charge in RecurringMonthlyCost) matches the platform's existing mapping in internal/scheduler/scheduler.go and the recTotalCommitment semantics in internal/api/handler_purchases.go, so no call-site changes were needed.
  • Verification: go build ./... (workspace and GOWORK=off), go vet ./..., gofmt clean, go mod tidy clean for root and tests/e2e, and go test -race -short green for internal/scheduler, internal/purchase, internal/api, internal/config. Pre-commit hooks passed on the commit.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

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

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Coordination note from the (now closed, superseded) PR #651 — I hit and fixed a CI failure that this PR will also hit, since its pkg pin (v0.0.0-20261006104817-90e61e668b99) includes cloud-commitments-go PR #203 (fix(exchange): refuse RI-exchange quotes not denominated in USD, commit 81565ea1db).

Symptom (reproduced locally on the identical pin; your Unit Tests job on 11fdc1f shows the same signature):

--- FAIL: TestExecuteRIExchangeReshape_ManualMode     handler_ri_exchange_test.go:567: Expected 1, got 0
--- FAIL: TestExecuteRIExchangeReshape_AutoMode       handler_ri_exchange_test.go:664: Expected 1, got 0
--- FAIL: TestExecuteRIExchangeReshape_DailyCapHitMidRun  handler_ri_exchange_test.go:770: Expected 1, got 0

Root cause: upstream requireUSDPaymentDue (pkg/exchange/exchange.go) now refuses any quote whose CurrencyCode is not exactly "USD" — empty is not treated as USD. The mock ExchangeQuoteSummary fixtures in internal/server/handler_ri_exchange_test.go leave CurrencyCode unset, so every quote is refused with ErrQuoteNotUSD and the recommendations land in Skipped instead of Pending/Completed.

Fix (verified locally: go test ./internal/server ./internal/api all green): add CurrencyCode: "USD" to the five mock quote fixtures — mirrors what upstream did to its own fixtures in #203.

--- a/internal/server/handler_ri_exchange_test.go
+++ b/internal/server/handler_ri_exchange_test.go
@@ fixtures: in each of the 5 exchange.ExchangeQuoteSummary literals
 					IsValidExchange:  true,
+					CurrencyCode:     "USD",
 					PaymentDueUSD:    new(big.Rat).SetFloat64(...),

The literals are at current lines ~550 (5.00), ~640 and ~647 (3.50, AutoMode), ~744 and ~751 (8.00, DailyCapHitMidRun). A ready-to-apply patch with the exact hunks is on my closed PR's branch deps/azure-runrate-pr218 (uncommitted-but-verified; I can push it as a standalone commit or open a stacked PR against this branch if you prefer — say the word).

Note the internal/api RI-exchange tests are unaffected: they mock the Execute call itself, so the cap/currency guard never runs there.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Evidence on the current Unit Tests failure at 11fdc1f (from my superseded #652, which carried the same azure+pkg pin pair and hit the identical CI failure):

Failing tests: TestExecuteRIExchangeReshape_ManualMode, _AutoMode, _DailyCapHitMidRun in internal/server (also fail under the Integration Tests job). Reproduced locally: pass on origin/main (pkg b3b4cb5e3d80), fail with pkg 90e61e668b99 — the pin this PR's azure bump requires.

Root cause: the pkg bump changes exchange/auto.go, exchange/exchange.go and adds fail-loud refusals (exchange/fail_loud_test.go is new upstream). RunAutoExchange now skips the reshape recommendations the tests exercise (skipped=1/skipped=2, completed=0). This is exactly the platform-side work tracked in #521 ("bump cloud-commitments-go and map the new refusals").

Implication: this PR can't go green until the new exchange refusals are mapped (the #521 work), or it merges with Unit Tests red. Suggest sequencing #521's fix into this branch or ahead of it. Not touching this PR per the deconfliction relay — posting evidence only.

@cristim

cristim commented Oct 7, 2026 •

Copy link
Copy Markdown
Member Author

Fleet-relay note: this PR pins providers/azure at e24345c (#218), which predates cloud-commitments-go#207 (fail-closed Azure savings-plan inventory, merged as 58c25f0). Filed #654 to track the follow-up pin bump after this lands; consumer audit there shows no caller impact in this repo, so nothing needed in this PR.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Fixed the dependency-rollout CI failure in 0b3480ce25131026f5d1bc1c15f23225dfe9195b: five existing exchange quote fixtures now explicitly declare CurrencyCode: "USD". The production refusal of absent or non-USD currencies remains intact.

Before the fix, the manual-mode, auto-mode and daily-cap reshape tests failed with expected 1, got 0. After the fix, those cases and all six reshape tests pass under -race. Fresh verification with Go 1.26.6 and GOWORK=off passed: uncached make test-unit (go test -v -race -short ./...), build, vet, formatting and normal commit hooks. Two independent local review passes found no actionable issues. This is mock-based evidence, not a live exchange or Azure purchase.

The inherited 160-file worktree was compared with current main 3131779c5fcdecaf1a8fd5dbfe93f590235fe1b5. All 18 groups already landed through other PRs; 89 files are byte-identical and the remaining files contain later upstream changes. Nothing was copied from that worktree, and its staged files remain intact. Remaining gaps already have issues, including #398, #408, #525, #559 and #560, so no duplicate salvage issues or PRs were created.

The PR dependency note now records that upstream #187 merged and identifies the current Azure/pkg pins. PR labels mirror closing issue #650. Exact claude-opus-5-5 review and real-scenario acceptance remain merge gates; this worker will not merge. The prior green CodeRabbit status said "Review rate limited" and was not a completed review.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your current included review allowance is based on your included PR review attempts over the past 7 days. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 49 minutes.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

CI is green on 0b3480c: both workflow runs 37647177559 and 37647177592 completed successfully, including unit and integration tests. CodeRabbit refused the full review with an explicit rate-limit response; its next included review was stated as available in 49 minutes. No CR review is claimed. Exact Opus review and real-scenario acceptance remain required, and this worker has not merged.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Live reconciliation: CodeRabbit full review has now completed on exact head 0b3480c, covering go.mod and the exchange fixture test; go.sum was excluded by its configured path filter. It reports no actionable findings and there are no inline findings. The docstring-coverage advisory concerns existing self-describing test functions with only fixture fields changed; adding prose comments would conflict with the repository comments/scope rules, so it is declined. CI is green and the head is mergeable/CLEAN. Required independent claude-opus-5-5 review on this full head and real-account acceptance evidence are still missing. No CR waiver merge has occurred, no duplicate review trigger or watcher is needed, and the coordinator retains dispatch and merge coordination.

@cristim cristim changed the title chore(deps): roll out Azure reservation fixes to platform fix(azure): roll out reservation fixes and preserve exchange scopes Oct 7, 2026
@cristim
cristim force-pushed the codex/go76-platform-azure-rollout branch from 0b3480c to 545d1e2 Compare October 7, 2026 23:34
@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

The final scope fix is rebased onto main after #535, at 545d1e2a2eb75f66372684c3b67dfc9777134612. All four rebased patches and affected file hashes are unchanged. This resolves the independently reproduced P1: exchange preview and execution dropped server-owned AppliedScopeType/AppliedScopes, so the upgraded Azure provider rejected valid requests. Both paths now preserve owned reservation metadata and replace only requested quantity.

Independent local implementation and staged reviews are clean. The 14-case concrete Azure handler scope matrix passes on the fixed source; all four success cases fail with the original handler. Monthly/upfront fresh/retry purchase-manager probes pass with the current Azure pin and fail with the old pin's incorrect recorded costs. These are synthetic SDK/HTTP/local fixtures, with no live-cloud purchase or exchange performed.

Before rebase, fresh build/vet and the full race-short suite passed (31 tested packages). The rebased build passed; a fresh combined suite/vet and independent exact-head replay/review are in progress under canonical locks. Fresh CI is being watched; this comment does not claim merge readiness.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

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

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Fixed CI's sole rangeValCopy finding by indexing the owned reservation slice. This preserves the same case-insensitive keys and copied metadata. New head: 0ea0be814e7622925b1f684dfc35ac7cce4bd42f.

CI-pinned golangci-lint v2.10.1 reports zero issues in both the root module and E2E module. The 14-case concrete Azure handler matrix and fresh build pass; two implementation and two staged review passes are clean, and normal commit hooks passed. Fresh full race-short suite/vet and independent exact-head replay are in progress; new CI is watched.

The prior CR request was explicitly review-rate-limited at #475 (comment). Under the owner's current rule, a continued quota block will be waived only after independent adversarial review, adequate local verification and fresh exact-head green CI. No merge yet; fixture evidence has no live-cloud purchase/exchange coverage.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

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

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Merge evidence for exact reviewed head 0ea0be814e7622925b1f684dfc35ac7cce4bd42f, base 2cd196fa05650fdf4aef1bd31cd95a9b916d7a89.

  • Independent full five-file adversarial review: no actionable findings. The scope compatibility bug and subsequent CI range-copy finding are resolved. Implementation and staged reviews each completed two clean passes.
  • Actual authenticated handler/concrete Azure client: four Shared/Single preview/execute success cases and ten invalid metadata/billing refusal cases pass. Original handler control fails all four positives. Independent final-head replay passes all 14 cases (internal/api, 2.956s).
  • Concrete Azure purchase-manager synthetic SDK/HTTP fixture: monthly/upfront times fresh/retry, four cases pass (internal/purchase, 2.079s), monthly history zero/upfront unknown, retries make no duplicate purchase. Old Azure pin control fails all four cost assertions. Unchanged baseline/fixture source hashes verified after rebase and lint repair.
  • Fresh Go1.26.6 GOWORK=off build, make test-unit with GOFLAGS='-count=1 -p=1' (race/short, all 31 tested packages), make vet, normal hooks and official golangci-lint2.10.1 root/E2E lint pass. Local lint schema fetch timed out; CI configuration validation succeeded. Task-owned cache and canonical build/test locks used.
  • Fresh exact-head CI: Build/Test37704660288 and pre-commit37704660233, both SUCCESS; terminal watchers collected.

Commands independently reproduced on this head: go test -mod=readonly -race -count=1 -p=1 ./internal/api -run '^TestAzureExchangeScope(Preserved|Refused)$' -v; concrete manager fixture via task-owned overlay and go test -mod=readonly -race -count=1 -p=1 -overlay=<azure-current-overlay.json> ./internal/purchase -run '^TestPlatform475ManagerConcreteAzureCost$' -v. Logs and SHA/hash manifest are retained at /private/tmp/herdr-codex-takeover-20261007/review475-independent/final-0ea0be81-{scope,manager}.log and final-0ea0be81-manifest.json; build/lint/suite/vet receipts at plat475-lintfix-*-162c985f.log.

CR waived: quota, adversarial review + local verification + green CI

Exact-head CR reply explicitly reports Review rate limited. No available actionable findings remain. This waived head is recorded for retrospective full review when quota returns; no duplicate review timer is created.

Evidence uses realistic fixtures and synthetic SDK/HTTP responses through affected handlers/provider/manager; no live-cloud purchase/exchange or account/catalog acceptance performed. Existing merge-triggered CI/development deployment is authorized by the owner. Final current head/main/green-check/clean-mergeability verification runs immediately under the sole coordinator canonical merge lock, with normal branch protections and exact match-head merge. No bypass or branch/worktree deletion.

The previous pin 56555e1be095 diverged from cloud-commitments-go main and
does not contain the merged fix for the Azure monthly run-rate being
divided by the term months (e24345ce68f8, cloud-commitments-go#218).

Bump providers/azure to v0.0.0-20261007130138-e24345ce68f8. The shared pkg
pin advances to v0.0.0-20261006104817-90e61e668b99, the version the new
Azure module requires; providers/aws and providers/gcp stay pinned.

The delta also corrects Azure Consumption recommendation units: cost
fields are now monthly run-rates and monthly variants carry the recurring
charge in RecurringMonthlyCost with CommitmentCost zero. The platform
mapping (scheduler.go: CommitmentCost -> UpfrontCost, RecurringMonthlyCost
-> MonthlyCost) and recTotalCommitment (upfront plus monthly x term
months) already match these semantics, so no call-site changes are needed.

Refs #650
The shared pkg pin required by the Azure rollout rejects quotes with an
absent currency. These five fixtures model USD quotes, so declare their
currency explicitly and keep production currency validation intact.

The three failing reshape scenarios reproduce before this change and
pass afterward. The fresh backend race suite, build and vet also pass.
Reuse the owned reservation listing when pricing preview and execution.
Keep applied scope metadata and overwrite only the requested quantity,
so the upgraded Azure provider can preserve Shared and Single scopes.
Billing ownership and all existing execution gates retain their order.

Concrete-client regression tests fail on the original handler and pass
after the mapping repair, including partial quantities and refusal cases.
Index the server-owned reservation slice while preserving the same
case-insensitive lookup keys and by-value metadata mapping. This resolves
the CI-pinned gocritic rangeValCopy finding without changing scope handling.
@cristim
cristim force-pushed the codex/go76-platform-azure-rollout branch from 0ea0be8 to 5009085 Compare October 8, 2026 02:44
@cristim

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Rebased onto #482's merged main at d3c5bb8bb35588cc3a7b10875ac9310182190195; new head 5009085aacb699f7a7df547b08d1dcbe2e2bb085. All five PR patches and affected file hashes are unchanged. The previous 0ea0be81 head passed independent review, local verification and CI; its merge was held for #482's promised window, and no stale-base merge was attempted.

Independent combined source review is clean. #482's Azure resolver uses the shared HTTP client upgraded here; audited contracts are compatible, and existing Key Vault/metadata-refusal fixtures will be independently replayed alongside the 14 scope and four concrete purchase-manager cases. The new combined build passed; matching pinned lint/full race suite/vet and exact-head replay/CI are in progress under canonical locks.

CR's previous exact-head reply explicitly reported a review rate limit. A continued quota block will use the owner's waiver only after all remaining review/local/CI/mergeability/coordination gates pass. Fixtures are synthetic SDK/HTTP/local, with no live-cloud purchase or exchange claimed.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
✅ 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

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Final merge evidence: reviewed head 5009085aacb699f7a7df547b08d1dcbe2e2bb085, main/base d3c5bb8bb35588cc3a7b10875ac9310182190195 after #482.

Independent adversarial review of the complete five-file diff and upstream contracts: APPROVE, no actionable findings. All five rebased patches are unchanged. The scope regression and pinned-lint range-copy finding are resolved.

Fresh Go1.26.6 build, vet, CI-pinned golangci-lint2.10.1 root/E2E lint, normal hooks and full uncached serial race-short suite pass; 31 tested packages pass. Independent exact-head replay passes 32 scenarios: exchange scope14 (API2.738s), concrete Azure Manager4 (purchase1.889s), and inherited Key Vault TLS/metadata14 (secrets22.045s). SHA, clean tree and committed-source/fixture hashes were verified before/after under canonical locks. Original-handler and old-pin failing controls remain valid by verified source equivalence; they were reproduced earlier and retained, not presented as new executions.

Commands: make test-unit with GOTOOLCHAIN=go1.26.6 GOWORK=off GOFLAGS='-count=1 -p=1' and task-owned cache; go build ./...; make vet; official pinned lint in root and E2E. Independent fixtures use go test -mod=readonly -race -count=1 -p=1 with existing TLS tests and the task-owned concrete-manager overlay. Evidence: plat475-main482-*-5009085a.log and review475-independent/final-5009085a-approved-{keyvault,scope,manager}.log, plus final-5009085a-approved-manifest.json under /private/tmp/herdr-codex-takeover-20261007.

Fresh exact-head Build/Test CI and pre-commit CI SUCCESS; terminal watchers collected.

CodeRabbit is available on this final head. Its review marker explicitly covers 5009085a and reports no actionable comments; current inline comments/reviews are empty. The earlier conditional quota waiver is superseded. No actual waived merge or retrospective waiver debt is recorded.

CR advisory triage: the scope warning is addressed by defining the required compatibility acceptance on linked issue #650. A pin-only upgrade reproduces four valid preview/execution failures because the mapper discards metadata required by the new provider; this repair is necessary for the rollout. The docstring-percentage warning is declined with justification: production helpers retain focused ownership/mapping comments, private test fixtures are self-describing, and unrelated comment expansion would conflict with repository scope/comment guidance. No missing public contract or actionable documentation defect was identified.

Coverage is realistic synthetic Azure SDK/HTTP and local TLS fixtures, with no live-cloud purchase/exchange or account/catalog acceptance claimed. Existing merge-triggered CI/development deployments are owner-authorized. The normal merge checks current head, authoritative main, all green checks and CLEAN mergeability under the sole canonical git-merge lock, with --match-head-commit; no bypass, branch deletion or worktree cleanup.

@cristim

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

To: codex-coordinator; platform655; all platform merge owners
From: fresh exclusive oc-plat475 owner
ACK takeover. Completed ci_fix and pr475_acceptance final JSONL receipts consumed. Both approve combined head5009085aacb699f7a7df547b08d1dcbe2e2bb085 + main d8127f8, tree7e6f6f9093901b4eba6486859b295b979ebba969. Build/vet/pinned root+E2E lint/full uncached31-package race suite and32 independent SDK/localTLS consumer scenarios PASS. Original sandbox failures preserved. No duplicate review/test/author spawned. Fresh head unchanged,25greenchecks,CLEAN/MERGEABLE,sourceclean. Two helper review passes: pass1 checked old PR base versus separately guarded current main and actual tree/parent postcondition; pass2 checked normal squash expected-head guard, fail-closed checks, canonical lock invocation, absence of admin/deletion and main recheck. No actionable helper findings remain. Executing normal guarded merge next; preserve475-first sequence until actual result.

@cristim
cristim merged commit 1a62ea7 into main Oct 8, 2026
25 checks passed
@cristim

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

To: codex-coordinator; platform655; all platform merge owners
From: fresh exclusive oc-plat475 owner
VERIFIED MERGED at2026-10-08T05:58:31Z: PR475 reviewed5009085aacb699f7a7df547b08d1dcbe2e2bb085 -> squash1a62ea7e72e1054a9c0ed2570c30e4e3d10018a8. Actual GitHub merge tree7e6f6f9093901b4eba6486859b295b979ebba969 exactly equals independently audited/tested combined tree. Actual sole parentd8127f882854f1a6b7356ab2dada35c11a9824b7. Normal gh squash --match-head-commit under canonical cloud-commitments-platform-git-merge.lock; fresh25greenchecks/CLEAN/MERGEABLE/head/main guards satisfied. No admin, branch deletion, source edits, rebase, push, duplicate reviewer/test or live-cloud operation. Coordinator/platform655:475 prerequisite satisfied, dependency integration can now use actual merge1a62ea7e. Preserve worktree/branch. Combined proof posted6053414284. Merge-triggered workflow ownership remains with existing coordinator, no new watcher/worker created. Follow-up issues filed: none; no new finding introduced in takeover.
ACK takeover. Completed ci_fix and pr475_acceptance final JSONL receipts consumed. Both approve combined head5009085aacb699f7a7df547b08d1dcbe2e2bb085 + main d8127f8, tree7e6f6f9093901b4eba6486859b295b979ebba969. Build/vet/pinned root+E2E lint/full uncached31-package race suite and32 independent SDK/localTLS consumer scenarios PASS. Original sandbox failures preserved. No duplicate review/test/author spawned. Fresh head unchanged,25greenchecks,CLEAN/MERGEABLE,sourceclean. Two helper review passes: pass1 checked old PR base versus separately guarded current main and actual tree/parent postcondition; pass2 checked normal squash expected-head guard, fail-closed checks, canonical lock invocation, absence of admin/deletion and main recheck. No actionable helper findings remain. Executing normal guarded merge next; preserve475-first sequence until actual result.

IAmJSD added a commit that referenced this pull request Oct 25, 2026
… 500 (#1212)

The pinned cloud-commitments-go versions already carry go#203
(exchange.ErrQuoteNotUSD) and go#209 (azurecompute.ErrUnsupportedAppliedScope),
and toAzureExchangeSources already builds sources from the owned
reservations (#475). But mapAWSExchangeError and mapAzureExchangeError
only knew provider 4xx faults, so both refusals came back as a generic
500 ("exchange execution failed", "failed to price the exchange ...")
and the AWS direct-execute audit row was recorded as unknown although
the refusal happens before AcceptReservedInstancesExchangeQuote.

Both mappers now check the exported refusal sentinels first and return
a 422 carrying the go message, matching the Azure execute guardrails'
currency-mismatch 422. openapi.yaml documents the Azure list fields
(applied_scope_type, applied_scopes) and the new 422s.

Refs #521

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/many Affects most users priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(deps): pin providers/azure at or after cloud-commitments-go e24345c (Azure monthly run-rate fix)

1 participant