[cristim]

What

PUT /api/config no longer overwrites per-service term, payment, coverage and ramp settings with the global defaults unless that default actually changed. When propagation fails, the request now returns an error and the global write is rolled back.

Why

propagateGlobalDefaults ran whenever the body contained any default_* key. It then copied all four defaults onto every service config and only logged save failures. Two real paths hit this:

• A coverage-only PUT reset aws/ec2 from 3-year all-upfront to the global term and payment, so the next scheduled purchase would buy the wrong commitment. This is the scenario in the issue.
• The recommendations lookback dropdown (frontend/src/recommendations.ts updateConfig(payload)) sends the whole cached config back. Every change to the lookback therefore reset term, payment, coverage and ramp on every service, and no per-service PUT followed to restore them.

A present-key overlay, which the issue suggested, does not fix the second path because every key is present in that body. So this PR propagates a default only when its value differs from the stored one. That matches what the Settings UI already does: it propagates a default to services only after the user changes it and confirms the diff preview.

How

• PostgresStore.UpdateGlobalConfigAtomic snapshots the config it read under the advisory lock. After the upsert, in the same transaction, it runs one UPDATE service_configs that sets only the columns whose default changed (a static SQL statement using CASE WHEN $changed THEN $value ELSE col END). If that statement fails, the global change rolls back too, so a retry is not stuck in a half-propagated state.
• The handler-side best-effort loop (propagateGlobalDefaults, anyKeyPresent) is removed. A store error now reaches the caller instead of a 200 updated.
• The two handler tests that asserted the old "succeed even if propagation fails" behaviour are replaced.

Verification

Env: GOTOOLCHAIN=go1.26.6 GOWORK=off GOFLAGS='-p=2 -count=1' AWS_EC2_METADATA_DISABLED=true

• New Postgres testcontainers test TestUpdateConfigPropagatesOnlyChangedDefaults (-tags integration), which drives updateConfig against a real store:
  • Setup: ec2 is set to 3 / all-upfront / weekly-25pct, rds to 1 / partial-upfront / monthly-10pct, and the global defaults to 1 / no-upfront / 80.
  • {"default_coverage": 70} keeps each service's term, payment and ramp and propagates coverage 70.
  • A full round-trip body that changes only the lookback leaves both services unchanged.
  • With a trigger that rejects service_configs updates, {"default_term": 3} returns failed to propagate global defaults... and the global default_term stays 1.
• On main source with the new tests: the test fails with expected: {3, "all-upfront", 70, "weekly-25pct"} actual: {1, "no-upfront", 70, "immediate"}. The new pgxmock and handler tests also fail on main.
• Mutation checks (each reverted afterwards):
  • Forcing termChanged := true fails the integration test and two pgxmock tests.
  • Discarding the propagation error fails the integration test and the pgxmock rollback test.
• go build ./..., go vet (with and without -tags integration) on api/config/server, and go test ./internal/api/ ./internal/config/ ./internal/server/ all pass (3629 tests). TestConfigInfrastructureKeyHTTP (integration) passes. golangci-lint run --build-tags integration on api and config reports no issues.
• The frontend is not touched.

For the reviewer to challenge

• Semantics: a client that re-sends an unchanged default to "reset all services" no longer gets that reset. No caller in the repo does this.
• Coverage still propagates when default_coverage changes, which is unchanged behaviour.

Closes #225

Summary by CodeRabbit

• Behavior Changes
  • When a global default changes, the new value is applied to the corresponding service configurations. Unchanged defaults and service-specific settings are preserved.
  • The global update and service changes are saved together. If updating service configurations fails, the global change is rolled back.
  • Submitting an unchanged configuration does not rewrite service configurations.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: f77f396e-6a45-4dd0-9fcb-9a456a15c816

📥 Commits

Reviewing files that changed from the base of the PR and between d9f3ceb and 6b49cfe.

📒 Files selected for processing (1)

• internal/api/handler_config_propagation_integration_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 2 reviews per hour.

---

📝 Walkthrough

Walkthrough

Global default changes now propagate to service configurations within the atomic database update. Only changed defaults update corresponding service-config fields. Propagation errors abort the transaction. The API handler no longer writes service configurations separately.

Changes

Global Default Propagation

Layer / File(s)	Summary
Transactional propagation internal/config/interfaces.go, internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go	UpdateGlobalConfigAtomic propagates only defaults that changed to service configurations in the same transaction. Propagation errors return an error and roll back the transaction.
API delegation and integration coverage internal/api/handler_config.go, internal/api/handler_config_test.go, internal/api/handler_config_propagation_integration_test.go	The handler removes separate service-config listing and saving. Tests cover changed and unchanged defaults, preservation of service-specific settings, transaction sharing, and propagation errors.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6b49c

No merge-blocking issue was found in the reviewed changes; the defaults update preserves unrelated service settings and fails atomically if propagation fails.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: propagate only changed global defaults within the same transaction.
Linked Issues check	✅ Passed	Issue #225 requires a PUT /api/config not to overwrite unrelated per-service settings and requires propagation failures to reach the caller. The PR propagates only defaults whose values changed, pre…
Out of Scope Changes check	✅ Passed	The handler, store, interface documentation, and unit and integration tests implement or verify issue #225. The incremental change only strengthens the relevant integration test. No unrelated changes …

• 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.}