[cristim]

Fixes red main. TestIdempotencyKeyCopyHasSingleWriter (added in #1816) pins the functions that may assign an execution's IdempotencyKey. #1798 moved the per-account derive out of Manager.executeForAccount into newAccountExecution (internal/purchase/execution.go:434), so the scan has failed on main since both PRs landed.

newAccountExecution is the only per-account writer on main. It sets idempotencyLineageKey(baseExec) + ":" + accountID. Both of its callers use that same derivation, so a skipped account's row carries the same stable key as its run:

• the per-account run, executeForAccountLate;
• the skipped-account row builder, skippedAccountRows.

No second writer was added. The other writers are unchanged: persistRetryExecution, the store's default mint and scan, and writeAttached.

This change moves only the pin. There is no production change. go test -run TestIdempotencyKey ./internal/purchase/ passes.

Part of #702.

🤖 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 7 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: fix/702-idempotency-writer-pin
• File: .coderabbit.yaml
• Commit: 60702c4
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: e4a709a7-912d-485d-90f1-d199dc306800

Commits

Reviewing files that changed from the base of the PR and between d459a8b and 60702c4.

Files selected for processing: 1

• internal/purchase/idempotency_key_writers_test.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

[cristim]

APPROVED 60702c4 by claude-cc-platform-2

Cold review. The change is test-only: one line in pinnedIdempotencyKeyWriters (internal/purchase/idempotency_key_writers_test.go:28).

Writers checked against the code at this head (git grep 'IdempotencyKey *=' and IdempotencyKey: outside tests):

• internal/api.Handler.persistRetryExecution: handler_purchases.go:2775.
• internal/config.PostgresStore.SavePurchaseExecutionTx: store_postgres.go:1156. This is the legacy empty-key fill.
• internal/config.scanExecutionRows: store_postgres.go:2272.
• internal/purchase.newAccountExecution: execution.go:437. Its callers are executeForAccount (:351) and the skipped-account rows (:477). Both derive the same stable lineage:accountID key, so the copy has one writer. executeForAccount no longer assigns the key.
• internal/purchase.Manager.writeAttached: plan_step_attach.go:331.

Checks

• The pinned set matches the five writers exactly, and the old Manager.executeForAccount entry is gone.
• Mutation: in a scratch worktree I added a second assignment, acctExec.IdempotencyKey = ..., inside executeForAccount after the newAccountExecution call. TestIdempotencyKeyCopyHasSingleWriter fails and reports the extra writer, so the scanner still catches a new writer. I reverted the change.

Local verification (macOS, at this head, throttled):

• go test -run TestIdempotencyKeyCopyHasSingleWriter ./internal/purchase/ passes.
• go test ./internal/purchase/ passes.

There's no production change. This fixes main's CI - Build failure from 672fb90 onward, where #1816 and #1798 each passed on their own base but fail together.

[cristim]

APPROVED 60702c4 by claude-ui-fix-1

This is a code review; runtime probes are queued and will follow.

Change. In internal/purchase/idempotency_key_writers_test.go, pinnedIdempotencyKeyWriters replaces internal/purchase.Manager.executeForAccount with internal/purchase.newAccountExecution. There is no production change.

Writer set at origin/main, from git grep 'IdempotencyKey *=':

• internal/purchase: exactly one per-account writer, newAccountExecution (execution.go:437: idempotencyLineageKey(baseExec) + ":" + accountID). fix(purchase): record fan-out accounts skipped before they started #1798 moved it there unchanged from executeForAccount (pre-fix(purchase): record fan-out accounts skipped before they started #1798 execution.go:309). Its two callers, executeForAccount (:351) and the skipped-account row builder (:477), derive the same stable per-account key.
• Outside purchase: unchanged. These are api.Handler.persistRetryExecution and config store_postgres.go:1156 (empty-key UUID default) and :2272 (scan).

The new pin matches the code, and the single-writer contract holds.

Pending probes: a second acctExec.IdempotencyKey assignment in executeForAccount should fail TestIdempotencyKeyCopyHasSingleWriter, and so should dropping the newAccountExecution assignment.

[cristim]

Follow-up runtime probes for my approval at 60702c4. These ran in a scratch worktree at that SHA, on the review lock, with GOMAXPROCS=4 and -p 2.

• TestIdempotencyKeyCopyHasSingleWriter passes at the head.
• Probe 1: adding a second acctExec.IdempotencyKey assignment in the account path fails the test. The writer set grows from 5 to 6 and names internal/purchase.Manager.executeForAccountLate.
• Probe 2: dropping the newAccountExecution assignment fails the test.

So the scanner still catches a second per-account writer and a missing one.