fix(server): SQS batch failure redelivers already-processed purchase messages (no BatchItemFailures) #108

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Where

• internal/server/lambda.go:226-248 (handleLambdaSQSEvent), specifically the single batch-level error return at internal/server/lambda.go:243-245
• internal/purchase/messages.go:113 (the execution not found path that produces the poison message)
• No BatchItemFailures or ReportBatchItemFailures anywhere in the repo: git grep over origin/main returns zero hits, in Go source and in the Terraform event-source mappings alike

What

The handler loops over sqsEvent.Records, collects failed message IDs, and then returns a single error for the whole batch. Lambda's SQS integration treats a non-nil error as "the entire batch failed", so every record in the batch, including the ones HandleSQSMessage already processed successfully, becomes visible again on the queue.

Partial batch responses are the mechanism that exists to prevent this, and the repo has neither half of it: no events.SQSEventResponse return type in the handler, and no function_response_types = ["ReportBatchItemFailures"] on the event-source mapping.

Failure scenario

A batch of 5 messages arrives. Messages LeanerCloud/cloud-commitments-cli#1 to LeanerCloud/cloud-commitments-cli#4 are approve messages that succeed and drive real purchase-execution state transitions. Message LeanerCloud/cloud-commitments-cli#5 references a deleted execution and fails with execution not found (internal/purchase/messages.go:113).

handleLambdaSQSEvent returns an error, so SQS redelivers all 5.

On redelivery, LeanerCloud/cloud-commitments-cli#1 to LeanerCloud/cloud-commitments-cli#4 hit checkAutoExecuteGate and the status CAS, and now fail with "not eligible" or a status mismatch, because they already ran. That re-fails the batch, which redelivers again, and so on until maxReceiveCount moves all 5 to the DLQ.

Two consequences. First, the operator investigating the DLQ sees four phantom failures for purchases that actually completed, which is the worst possible signal during a money-path incident: the obvious remediation (replay the DLQ) is the wrong one. Second, the single poison message keeps the whole batch churning through the redrive budget, so genuinely retryable failures in the same batch burn their retries on someone else's poison.

Idempotency downstream is the only thing keeping this from being a straight double-spend: the CAS on the execution status is what turns the redelivery into a no-op rather than a second purchase. Any handler reachable on this path that is not CAS-guarded turns the same redelivery into a duplicate money action.

Fix direction

1. Return events.SQSEventResponse{BatchItemFailures: [...]} containing only the message IDs that actually failed, and return a nil error.
2. Set function_response_types = ["ReportBatchItemFailures"] on the SQS event-source mapping in the Terraform module, or the response is silently ignored and the behaviour does not change.
3. Regression test: a batch where record N fails must leave records 1..N-1 acknowledged, asserted on the returned BatchItemFailures set rather than on the error value.

Point 2 is the easy one to miss: shipping only the Go change looks correct in review and in unit tests while behaving exactly as it does today in production.