[IAmJSD]

Summary

This is the backend half of Stage 1 for service configs. service_configs has had created_at and updated_at columns since migration 000001, but no read selected them. That left the API with no way to say when a Purchasing service config last changed. Override rows already show their stamp (#1314). This PR does the same for service config rows on the backend side.

Refs #8

Root cause

config.ServiceConfig had no timestamp fields. All three service_configs reads (GetServiceConfig, the FOR UPDATE read inside UpdateServiceConfigAtomic, and ListServiceConfigs) selected only the settings columns.

Fix

• internal/config/types.go: ServiceConfig gets read-only CreatedAt / UpdatedAt (*time.Time, json:"created_at,omitempty" / json:"updated_at,omitempty"). A nil value is omitted from JSON.
• internal/config/store_postgres.go:
  • serviceConfigColumns now includes created_at, updated_at. All three reads scan through one new scanServiceConfig, so ListServiceConfigs no longer keeps its own copy of the column list and scan. The existing comment already mentioned a scanServiceConfig that didn't exist.
  • The upsert in saveServiceConfigWith now ends with RETURNING created_at, updated_at and sets both on the saved struct. So UpdateServiceConfigAtomic returns the stamps of the row version it actually wrote, not the pre-write updated_at its merge copied from the locked read. A caller-supplied stamp never survives a save. No column is written from the struct; the stamps come only from the column defaults and the existing update_service_configs_updated_at trigger.
• internal/api/handler_config.go: PUT /api/config/service/{provider}/{service} clears both stamps right after it decodes the body. A client that sends back the GET response cannot get them into the merged row. This matters on a first save, where mergeServiceConfig returns the body as is.
• internal/config/store_postgres_plan_writes.go: new marshalPlanServices drops the stamps from each entry before the plan's services JSONB is written. It is used by both the create and update paths. A plan's services map holds a copy of settings, not a service_configs row, so a stamp there could only be a value the caller supplied.
• internal/api/openapi.yaml: ServiceConfig.created_at / updated_at are documented as readOnly date-time fields, present on stored rows and left out of plan services entries.

Other write paths checked, no change needed:

• PUT /api/config decodes into GlobalConfig, which has no services field.
• The override import (handler_override_import.go) builds ServiceConfig from typed columns only, to run the combo check.
• PlanRequest.buildServiceConfig builds plan services from scalar request fields.

No migration is needed.

Concurrency

The read side adds no locking. Writes keep the existing advisory lock plus row lock (#1488); the only change is that the upsert now reports what it wrote. TestUpdateServiceConfigAtomic_ConcurrentSavesReportTheirOwnStamps runs 8 concurrent atomic updates of one row on real Postgres. It checks that:

• created_at never moves;
• every reported updated_at is at least as new as the seed's;
• the row left in the table carries exactly the updated_at that its last writer was given.

Verification

All evidence below is local: unit tests against pgxmock and the mock store, plus integration tests against real Postgres (testcontainers). Go 1.26.9 with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true.

go build ./...                                   # ok
go vet ./... && go vet -tags integration ./...   # ok
go test -count=1 ./internal/config/              # ok
go test -count=1 ./internal/api/                 # ok
go test -count=1 ./internal/mocks/               # ok
golangci-lint run --allow-parallel-runners ./internal/config/ ./internal/api/                         # 0 issues
golangci-lint run --allow-parallel-runners --build-tags integration ./internal/config/                # 0 issues
gocyclo -over 10 -ignore "_test\.go" internal/config/types.go internal/config/store_postgres.go \
  internal/config/store_postgres_plan_writes.go internal/api/handler_config.go                        # nothing over 10
go test -tags integration -count=1 -timeout 20m \
  -run 'TestServiceConfigStamps_ReadsAndSavesAgree|TestUpdateServiceConfigAtomic_ConcurrentSavesReportTheirOwnStamps|TestPurchasePlanServices_StampsNeverStored' \
  ./internal/config/                             # 3 PASS (real Postgres)

New and updated tests:

• Unit: TestPGXMock_GetServiceConfig_Success and TestPGXMock_ListServiceConfigs_Success (stamps scanned per row); TestPGXMock_UpdateServiceConfigAtomic_LockedReadMergeWrite and TestPGXMock_UpdateServiceConfigAtomic_NoRowPassesNil (the returned row carries the RETURNING stamps, not the read's or the caller's); TestMarshalPlanServices_DropsStampsWithoutMutatingInput, TestPGXMock_CreatePurchasePlan_StoresServicesWithoutStamps, TestPGXMock_UpdatePurchasePlan_StoresServicesWithoutStamps; TestGetConfig_ServiceRowsCarryStamps (RFC 3339 on the wire, omitted when nil); TestUpdateServiceConfig_IgnoresClientStamps.
• Integration: the three tests named in the command above.

Fail-before / pass-after:

• Fail-before: with the four production files reset to origin/main, the new tests do not compile (cfg.CreatedAt undefined), because the fields do not exist on main.
• Mutation probes: each one was applied on its own to this branch, and the named tests failed. Each was then reverted and the tests pass again.
  1. Reads drop the scanned stamps: TestPGXMock_GetServiceConfig_Success, TestPGXMock_ListServiceConfigs_Success and integration TestServiceConfigStamps_ReadsAndSavesAgree FAIL.
  2. The save ignores its RETURNING values: both UpdateServiceConfigAtomic pgxmock tests, TestServiceConfigStamps_ReadsAndSavesAgree and the concurrent integration test FAIL.
  3. Plan services marshalled with stamps: the three plan unit tests and integration TestPurchasePlanServices_StampsNeverStored FAIL.
  4. The handler keeps the body's stamps: TestUpdateServiceConfig_IgnoresClientStamps FAILS.

Not run here: existing integration tests other than the three above, including handler_config_service_race_integration_test.go and store_postgres_service_config_atomic_integration_test.go. They exercise the upsert that changed from Exec to QueryRow ... RETURNING; they are left to the combined-tree run.

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ 400d256b = origin/main 7b86e386 plus these heads, merged in this order:

Branch	Tested head
fix/issue-243-rest	007c163
fix/issue-1206	312072f
fix/issue-111-email	6d422e2
fix/issue-111-fail-closed	3817e29
fix/issue-712-age-cap	a91120b
fix/issue-712-fargate	aeaf0d4
fix/issue-613-legacy-ids	350e837
fix/issue-120-guards	6f38ebc
fix/issue-120-shared-predicate	cb25735
fix/issue-151-account-update-keep	d20cf01
fix/issue-713-upcoming-filter	e73fdfb
fix/issue-263-bastion-opts	7861f32
fix/issue-43-fixture-ids	8cee203
fix/issue-177-makefile-walk	365a9d6
fix/issue-27-seller-not-registered	d845f66
fix/issue-313-user-columns	6d759dd
fix/issue-353-mock-panics	aac2edf
fix/issue-8-config-stamps	b548ae5
fix/issue-30-monthly-totals	f637921

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers. The full suite ran on f7c48020, which differs from this tree only by test-only commits on fix/issue-30, fix/issue-713 and fix/issue-263 that replace placeholder account-number literals with named constants:

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests, every changed package that has them, one package at a time (-tags integration -count=1): internal/analytics, internal/api, internal/auth, internal/config, internal/database/postgres/migrations, internal/purchase, internal/scheduler and internal/server all pass. Migrations 000149, 000150 and 000151 apply in order on this tree.
• Frontend: the changed jest files (approval-details, settings-accounts, api) pass (221 tests).
• Secrets: the first run flagged 9 new fixture lines (placeholder account numbers next to an account key); fixed with named constants. Re-run on this tree: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.
• Terraform .tftest.hcl files were not run locally (CI runs them). Nothing was applied to any cloud.

Out of scope / notes for reviewers

• Stage 2 (the audit-log table, endpoint and history UI) is not part of this PR, and the issue stays open for it.
• The stamp can bump without a visible change. This is existing write behavior, left unchanged here:
  • propagateChangedDefaults runs its UPDATE service_configs with no WHERE clause. Every row's updated_at moves whenever a global default changes, including rows whose values the CASE leaves alone (for example Azure/GCP rows when only the AWS payment default changes).
  • The upsert also sets updated_at = NOW() on a save that changes nothing.
  • A possible follow-up is a WHERE ... IS DISTINCT FROM guard on both. It is deliberately not done here because it changes which rows the propagation locks.
• NOW() is the transaction start time. When two serialized writers queue on the advisory lock, the later one to commit can get the earlier updated_at, by the length of the lock wait (milliseconds). The trigger overrides any explicit value, so switching to clock_timestamp() would need a migration. The concurrent test checks that each writer gets its own stamp. It does not check that stamps increase in commit order.
• Existing test reaches the network (unrelated). TestHandler_HandleRequest_GetConfig (handler_test.go), run on its own, makes STS/IMDS calls through resolveSourceIdentity and trips the no-network guard (internal/api tests call real AWS Cost Explorer and probe IMDS #855). The new GET test avoids this with handler.sourceIdentityOnce.Do(func() {}).

Frontend follow-up for the UI owner

• Settings → Purchasing: each service config row can show Last updated: <updated_at> in UTC, matching the override rows from feat(frontend): show when each account service override was updated #1314. The data is GET /api/config, services[i].updated_at, an RFC 3339 string, omitted when unknown, so render nothing in that case. created_at is also available if wanted.
• Saving: the PUT ignores both fields, so sending back the full row is harmless. After a save, re-fetch GET /api/config (or GET /api/config/service/{provider}/{service}) to get the new stamp; the PUT response is still {"status":"updated"}.
• Tests: the issue's acceptance criteria ask for settings.test.ts to assert that the stamp is rendered when the API returns it.

Maintainers: please mirror the issue's triage labels onto this PR: enhancement, effort/l, triaged, priority/p3, severity/low, urgency/eventually, impact/few, type/feat.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

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: 7b86e38
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9405bd7e-0553-4004-9f06-8fc48f4fd8f9

Commits

Reviewing files that changed from the base of the PR and between 7b86e38 and ad99548.

Files ignored due to path filters: 1

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

Files selected for processing: 11

• go.mod
• internal/api/handler_config.go
• internal/api/handler_config_stamps_test.go
• internal/api/openapi.yaml
• internal/config/store_postgres.go
• internal/config/store_postgres_pgxmock_test.go
• internal/config/store_postgres_plan_writes.go
• internal/config/store_postgres_service_config_atomic_test.go
• internal/config/store_postgres_service_config_stamps_integration_test.go
• internal/config/store_postgres_service_config_stamps_test.go
• internal/config/types.go

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{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 ad99548533194f0939712c7d7bc0a8c439951f8f

Head moved from b548ae52 to ad995485 during review. The only new commit bumps golang.org/x/net to v0.62.0, the same change as #1564. I re-checked build, vet and the integration runs below at the new head; the verdict is unchanged.

Blocking:

• none

Non-blocking:

• updated_at also moves when a changed global default is copied onto every row (propagateChangedDefaults, via the update trigger). The openapi description says so. A UI showing "Last updated" should know a global edit bumps every row.
• Issue ux(settings): expose updated_at + audit log for service overrides and configs #8's Stage 1 is UI-facing (rows show "Last updated" and frontend tests). This is the backend half, and Refs #8 is correct.

Attacked:

• Write paths: the PUT handler clears both stamps after decoding the body. The upsert stamps come only from RETURNING. marshalPlanServices strips stamps on plan create and update, and covers mergeRequestedServices/overlayServiceConfig. The override import builds a ServiceConfig from typed columns only. I grepped for reflect.DeepEqual/cmp.Equal on ServiceConfig (none, so new pointer fields can't turn a no-op into a change) and for strict decoders in this repo and the cli/go/mcp repos (none on this type). ConfigResponse.Services gains two omitempty fields, which is backward compatible.
• Ran go build ./...; go vet with and without -tags integration; go test -race -short on config and api (PASS); go test -race -tags integration ./internal/config/ (PASS, 267s). My first run of that package failed on a 743s run at load average ~47, then passed twice once load was under 15, so I'm treating it as load, not the PR. -tags integration -run 'ServiceConfig|Config_' ./internal/api/ fails only on the internal/api tests call real AWS Cost Explorer and probe IMDS #855 outbound-network guard (IMDS calls), and fails identically on origin/main, so that's not from this PR. golangci-lint on config/api did not finish within my time limits (stopped after 30 minutes and 500 seconds), so lint is left to CI's lint job.
• Fail-before (mutation): with the handler's stamp-clearing line removed, TestUpdateServiceConfig_IgnoresClientStamps fails. With saveServiceConfigWith no longer assigning its RETURNING values, both TestPGXMock_UpdateServiceConfigAtomic_* tests fail. Both restored.

Body-stale:

• none

_{Reviewer: rG2. 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 ad99548533194f0939712c7d7bc0a8c439951f8f merged onto main (d05a1726), together with #1547, #1555, #1558, #1559, #1561, #1562 in that order (combined tree eea0132d, 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 1 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: 1 approving independent review(s) on ad995485, green CI, and the combined-tree verification above. Squash-merged at exactly ad99548533194f0939712c7d7bc0a8c439951f8f.