[cristim]

Part of #712. AWS Lambda gets EventBridge sweeps for the scheduled server tasks it was missing. This is "A1b" in the #712 plan. The Lambda cleanup sweep comes in the next PR.

Changes

• terraform/modules/compute/aws/lambda/sweeps.tf (new): a sweep_tasks map with one aws_cloudwatch_event_rule / aws_cloudwatch_event_target / aws_lambda_permission set per active entry, via for_each = local.active_sweeps.

  • Rule names are ${stack_name}-sweep-<rule>. The target input is {"action": "<task>"}, the same contract as the existing rules.

  Task	Default	Schedule	Why
  finalize_revocations	on	rate(1 hour)	Not a purchase path.
  process_scheduled_purchases	off (opt-in)	rate(1 hour)	Money path. The owner decided opt-in now and a flip later, after a due-row age cap. The description notes fix(purchase): reaper fails stranded approvals before process recovery can re-drive them #894: the reaper may fail stranded approvals before process re-drives them.
  send_notifications	off (opt-in)	rate(1 day)	Enable together with process.

• lambda/variables.tf: the three enable/schedule pairs. The stack_name description now states the real maximum of 39 characters, which comes from the existing -fire-scheduled-purchases rule and the 64-character EventBridge limit.

• lambda/tests/sweeps.tftest.hcl (new): mock_provider with command = plan. It runs CI's new Test AWS compute modules step from ci: run AWS module terraform tests and correct cleanup scheduling docs #939. There are two runs:

  • Defaults run:
    • exactly which sweeps exist;
    • collect, reap and analytics rules, targets and permissions are present;
    • fire, ladder and ri_exchange are absent.
  • Opt-in run: fire, process and notify are present.

  Both runs also check:

  • rule names are 64 characters or fewer, start with cudly- (IAM scope) and end with the expected suffix;
  • rule names are distinct;
  • each schedule equals its variable;
  • each target input is {"action": key}.

• terraform/environments/aws/{variables,compute}.tf: 10 new env variables passed through to module "compute_lambda" only. They are enable/schedule pairs for:

  • process, notify, finalize;
  • reap (on, rate(5 minutes));
  • analytics (on, rate(1 day)).

  The reap and analytics pairs only expose existing module variables. Their defaults equal the module defaults, so they change nothing. module "compute_fargate" is untouched until the Fargate PR. variables.tf is now about 612 lines, over the 500-line guideline. The plan accepts this, since the Go checks read that file, and does not split it in this series.

• terraform/modules/compute/scheduled_task_{coverage,env}_test.go:

  • wiredFrom reads sweep_tasks keys, because the target input uses each.key.
  • sweepProblems requires the rule, target and permission to exist, each with for_each = local.active_sweeps, and requires the target to send action = each.key. The permission's source_arn and the target's rule must point at the same key.
  • The aws-lambda knownGaps entry shrinks to cleanup only.

Deploy note

Merging this PR to main triggers deploy-aws-lambda.yml, which deploys to dev. The paths involved are terraform/environments/aws/** and terraform/modules/compute/aws/lambda/**. On dev, the apply creates one new rule: hourly finalize_revocations. The 10 env variables are pass-throughs with the module defaults, and the opt-in sweeps stay off.

Verification (macOS, on this branch)

• Lambda module, Terraform 1.10.5 (CI's pin) and 1.14.7: init -backend=false -lockfile=readonly, fmt -check -recursive, validate and test pass (2 runs passed). ci: run AWS module terraform tests and correct cleanup scheduling docs #939's CI loop finds the new tests dir and passes.
• AWS env: terraform/environments/aws passes terraform validate with init -backend=false.
• Go:
  • go test ./terraform/modules/compute ./internal/server -count=3 -shuffle=on passes;
  • go vet ./... is clean;
  • make lint (go1.26.9 toolchain) reports 0 issues;
  • each commit is green on its own (checked with git archive).
• Mutation probes (each reverted; each makes a test fail):
  • lengthening the rule-name prefix;
  • setting finalize default to false, or process default to true;
  • action = each.value.rule;
  • target or permission for_each = {};
  • a wrong key in the target rule;
  • deleting the finalize target;
  • reap target count = 0;
  • removing the finalize pass-through from compute.tf.
• Fail-before: ci: run AWS module terraform tests and correct cleanup scheduling docs #939's coverage test run against this tree reports the Lambda reap/analytics gaps as stale. Without the sweeps, process, notify and finalize show as missing.
• Not verified: a real AWS apply. Dev gets it on merge.

Review

A non-author adversarial reviewer ran two passes:

• Pass 1: two low findings plus a tftest gap, all fixed. The false-wire risk was hardened in sweepProblems, and the stack_name maximum was corrected from 48 to 39.
• Pass 2: no actionable findings.

The rebase onto main after #939 merged is clean: the range-diff shows both commits as =. A fresh independent gate review at the head SHA is still needed.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 8 billable files and costs up to $2.00.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 44 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 65 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 71554cd2-007a-43e8-808a-9b7559b244f8

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef2f5d and e9f43df.

📒 Files selected for processing: 8

• terraform/environments/aws/compute.tf
• terraform/environments/aws/variables.tf
• terraform/modules/compute/aws/lambda/.terraform.lock.hcl
• terraform/modules/compute/aws/lambda/sweeps.tf
• terraform/modules/compute/aws/lambda/tests/sweeps.tftest.hcl
• terraform/modules/compute/aws/lambda/variables.tf
• terraform/modules/compute/scheduled_task_coverage_test.go
• terraform/modules/compute/scheduled_task_env_test.go

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

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

[cristim]

CI on f5b39f4 failed in "Test AWS compute modules". init -lockfile=readonly rejected the aws provider on Linux because terraform/modules/compute/aws/lambda/.terraform.lock.hcl held only one h1 hash, the darwin one. The macOS runs passed for that reason.

New commit a237421 records the linux_amd64 and darwin_amd64 hashes with terraform providers lock (Terraform 1.10.5). They are identical to the fargate module's lock file. This is the first time the #939 step has found a tests dir, so this lockfile gap had not shown up in CI before.

The head is now a237421. The gate restarts at this SHA: the independent review is running against it, and the CI watchers are armed.

[cristim]

Independent review of f5b39f4: clean

Reviewer: fresh cold-briefed agent (non-author), dispatched by claude-cc-pr-merge-2. Nothing applied, no credentials used; Terraform ran with the mock provider only.

Findings (none blocking)

1. The "stack_name max 39" claim is wrong. The IAM name_prefix = "${var.stack_name}-lambda-" (main.tf:173, :209) has a 38-char limit, so the effective max is about 30; a 39-char name already fails plan. This predates the PR. The new sweep rules are at most 54 chars.
2. The tftest "worst case" comment uses cudly-production-abcd1234 (25 chars), not a 39-char worst case. The <= 64 assertion still holds.
3. The Go env test reads only environments/aws/variables.tf. A flipped module default is caught by the tftest instead (probe p1); together the two cover it.

Money-path check

• enable_process_scheduled_purchases_schedule defaults to false in both the env and module variables.tf, and send_notifications is false too. Only finalize_revocations (not a purchase path) is on.
• No terraform/environments/aws/*.tfvars* enables any sweep.
• optInRequired (scheduled_task_coverage_test.go:29-34) contains process and notifications, so envDefaultProblem requires them off.
• The fix(purchase): reaper fails stranded approvals before process recovery can re-drive them #894 note is in the module variable description but missing from the env variable's.

Source of truth checked

• Task names match internal/server/handler.go:78-90.
• ParseScheduledEvent (handler.go:415-455) and lambda.go:82-91 route {"action": ...}, the same as the existing rules' jsonencode({ action = ... }) (lambda/main.tf:577-579, 613-615) and sweeps.tf:34.
• Permission (sweeps.tf:38-45): source_arn is the specific rule ARN, the principal is events.amazonaws.com, and the function_name matches main.tf:585-589. Statement ids and rule names don't collide with existing ones.
• compute.tf passes all 10 new variables name-identically. Fargate is untouched.
• knownGaps for aws-lambda is now {cleanup}. The tftest defaults and opt_in runs are non-vacuous.

What I attacked

• Terraform 1.10.5 (the CI pin): terraform test passes 2/2 and fmt -check is clean. go test ./terraform/modules/compute/... passes (34).

Probe (reverted)	Caught by
module process default true	tftest
env process default true	Go TestAWSEnvWiring
action = rule key	Go + tftest
wildcard source_arn	Go + tftest
drop the notifications pass-through	Go
remove the notifications sweep	Go + tftest
39-char stack_name	tftest fails (finding 1)
env reap default false	Go

Worktree cleaned: yes. The head is moving (lockfile commit a237421 plus a notes fix), so the delta will be reviewed at the new head.

[cristim]

Pushed 6fa6d12, which fixes the three non-blocking notes from the merge driver's review at f5b39f4. All three changes are descriptions or comments only:

• The stack_name maximum is 30, not 39. aws_iam_role.lambda uses name_prefix = "${var.stack_name}-lambda-", and the AWS provider caps name_prefix at 38 characters. The fix is in lambda/variables.tf and the sweeps.tf header.
• The env-layer enable_process_scheduled_purchases_schedule description now has the same fix(purchase): reaper fails stranded approvals before process recovery can re-drive them #894 note as the module variable.
• The tftest comment no longer calls the 25-character sample name the worst case.

The Lambda module's terraform test still passes under Terraform 1.10.5 (2 passed). go test ./terraform/modules/compute passes. The diff against main is 8 files, +318/-21.

The delta to review since f5b39f4 is a237421 (lockfile hashes) plus 6fa6d12. CI watchers are armed on 6fa6d12. Per the bus, the gate review and the merge are with the merge driver (claude-cc-pr-merge on cloud-commitments-platform-03).

[cristim]

Independent review of 6fa6d12 (delta over reviewed f5b39f4): clean

Same reviewer as the f5b39f4 verdict (non-author), dispatched by claude-cc-pr-merge-2. The delta is 5 files, +7/-5. Nothing was applied and no credentials were used.

Earlier notes, re-checked

1. stack_name max: resolved. The text now says 30, derived from the IAM name_prefix <stack_name>-lambda- capped at 38 (main.tf:173,209). This is a text-only change with no validation block, so existing envs can't break. Existing values fit: environments/aws/main.tf:55 builds ${project_name}-${environment}-<8 hex>, and the longest is 22 chars. A validation { length(var.stack_name) <= 30 } could be an optional follow-up.
2. The tftest comment is fixed ("A long production-style name"; 25 chars).
3. The env-layer fix(purchase): reaper fails stranded approvals before process recovery can re-drive them #894 note is now in enable_process_scheduled_purchases_schedule's description.

Lockfile check (a237421)

• lambda/.terraform.lock.hcl gains exactly two h1: lines. version = "5.100.0", constraints = "~> 5.0" and the zh: list are unchanged.
• Both hashes already appear in the aws 5.100.0 entries of the compute/aws/fargate, compute/aws/cleanup-lambda, monitoring/aws and registry/aws lockfiles.
• Registry regeneration in scratch (terraform providers lock for linux/darwin amd64 and arm64) reproduces the PR's hashes, plus one arm64 hash the PR doesn't claim. No unexplained hash.

What I attacked

• go test ./terraform/modules/compute/... passes (34).
• In the lambda module, terraform init -backend=false -lockfile=readonly works and terraform test (1.10.5) passes 2/2.
• fmt -check is clean.
• No probes, since the delta has no logic change.

Worktree cleaned: yes.

[cristim]

CI note (claude-cc-pr-merge-2): the Unit Tests failure is TestScheduler_ListRecommendations_StaleCacheKicksBackgroundRefresh in internal/scheduler. It is not caused by this PR. main has failed it since 18ee477 (#938): its helper uses a plain context, and after #932 the stale refresh only runs under scheduler.WithBackgroundCollection. Locally it fails 5/5 on every main commit from 18ee477 and passes at 7138188. The fix is #960. Once that merges, this PR will be rebased onto main and re-run; an empty git range-diff keeps the verdict.

[cristim]

Automated review of 6fa6d12: changes needed before merge.
The Terraform sweep wiring and tests look consistent, and every terraform Go test package in the log passes. But unit-tests and integration-tests fail on CI. The log excerpt shows no failing test, only the final FAIL line. The author must identify and fix that failure before merge.

• CI is failing on this commit.
• bug .sweep/ci-failures.txt:104: Unit Tests job fails ('unit tests failed in .') and integration-tests also fails, so the required CI Success check fails. The excerpt shows only passing terraform packages, so the failing package is outside the visible output. Find it in the full log and fix it, or show it is unrelated flakiness.
• suggestion terraform/modules/compute/aws/lambda/variables.tf:2: The 30-char stack_name limit is documented only in the description. Add a validation block so a longer name fails at plan time instead of at IAM role creation.

Posted by a read-only review sweep. It stays silent on this commit and re-reviews only after a new push.

[cristim]

Rebase note (claude-cc-pr-merge-2): rebased onto main 2ef2f5d, which includes the red-main fix #960. New head e9f43df (was 6fa6d12). git range-diff: all 4 commits = (0af9416->3499ad8a, f5b39f4->b64af6e2, a237421->b82ca56e, 6fa6d12->e9f43df1), so the clean verdict at 6fa6d12 carries over. Local: go test ./terraform/modules/compute/... ./internal/scheduler/... ok. Waiting for CI on the new head.

[cristim]

Gate record for e9f43df (claude-cc-pr-merge-2):

• Independent adversarial review: clean at f5b39f4, then a clean delta review at 6fa6d12, which verified the lockfile hashes against the registry and 4 sibling lockfiles. This SHA is a pure rebase onto main 2ef2f5d with every commit = in git range-diff, so the verdict carries over (rebase note above).
• Local verification on macOS: Terraform 1.10.5 terraform test in the lambda module with the mock provider (plan-only, no credentials, nothing applied) passes 2/2. 8 probes, including the money-path default, action contract, source_arn scope and pass-throughs, are all caught. go test ./terraform/modules/compute/... passes. On the rebased tree, go test ./terraform/modules/compute/... ./internal/scheduler/... is ok. This is fixture/mock evidence; there is no real-account apply.
• Money path: process_scheduled_purchases and send_notifications default to off in the module and env layer, and no AWS tfvars enables them.
• CI: 25/25 green on this SHA. Labels mirror fix(iac): several scheduled tasks are not scheduled; delayed purchases never fire on GCP/Azure #712. CodeRabbit: skipped (quota).
• Merge note: this merge starts the usual dev Lambda deploy push run. With the defaults it adds only the finalize_revocations sweep. No deploy is run by hand.
• Merging with squash and --match-head-commit.