[IAmJSD]

Summary

Acting-owner decision on #357 (IAmJSD, 2026-10-29): delete cmd/cleanup-lambda and everything that exists only to build or deploy it.

Root cause

cmd/cleanup-lambda deleted purchase_executions rows only when expires_at < now (plus the terminal-status guard from #1173). The only writer of expires_at is SavePurchaseExecution, via timeFromTTL(execution.TTL), and no production code path sets PurchaseExecution.TTL. So the column is NULL on every row current code writes, and the job never deleted an execution. It was also never deployed:

• None of its three deploy modules (compute/aws/cleanup-lambda, compute/gcp/cleanup-function, compute/azure/cleanup-function) is instantiated by any environment root. git grep finds no module block that sources them.
• No Dockerfile or Makefile target builds the /app/cleanup-lambda binary the AWS module's image_config.command points at. The Dockerfile builds only ./cmd/server and migrate.
• The GCP module sets entry_point = "cleanupExpiredRecords" on a go121 Cloud Function, and the Azure module POSTs {"dryRun": false} to a custom-handler Function App. cmd/cleanup-lambda is a package main that calls lambda.Start, so it could serve neither one.

The in-app cleanup scheduled task (handleCleanupExpiredRecords, which calls CleanupExpiredSessions and CleanupOldExecutions(DefaultExecutionTTLDays)) already handles session and execution retention with the live-status and health-score guards. Every platform schedules it (see "Database Cleanup Jobs" in docs/DEPLOYMENT.md).

What changed

Deleted:

• cmd/cleanup-lambda/ (main.go, main_integration_test.go)
• terraform/modules/compute/aws/cleanup-lambda/
• terraform/modules/compute/gcp/cleanup-function/: nothing consumes it, and it deploys the same dead job (the same {"dryRun": false} body and cleanupExpiredRecords entry point), so it goes too.
• terraform/modules/compute/azure/cleanup-function/: the brief did not name this one. It is the Azure copy of the same dead path: no consumer, and it POSTs the same {"dryRun": false} CleanupEvent body to /api/cleanup. It is deleted on the same reasoning. If you want it kept, the module deletion reverts cleanly on its own.
• terraform/modules/compute/gcp/cleanup_function_source_test.go (it only read the deleted GCP module)

References removed or repointed:

• scripts/test-azure-kv-access-policy.sh and scripts/test-gcp-secret-scope.sh: removed the cases that scanned the deleted modules. The AKS case and the default-scan-root cases still pin the #1621 and #1614 regressions. History comments in scripts/check-azure-kv-access-policy.sh, scripts/check-gcp-secret-scope.sh and the access-policy fixture now say the module was deleted.
• .trivyignore: removed AVD-AZU-0004, AVD-AZU-0012 and GCP-0001. Each justification named only the deleted modules, and git grep finds no other azurerm_*function_app, azurerm_storage_account or google_storage_bucket resource under terraform/ or iac/.
• .gitignore: removed the cleanup-lambda binary entry.
• docs/DEPLOYMENT.md: removed the module from the tree and the two "not consumed by any environment" notes.
• known_issues/33_*.md and known_issues/35_*.md: added chore(pkg/cmd): medium/low findings from the 2026-07-28 full review [platform part] #357 amendment/update notes. Neither file moves to resolved/, because 35 still has the open feat(plans): settle a failed-with-submission row so a managed plan can be deleted #999 entry and 33 tracks fix(iac): several scheduled tasks are not scheduled; delayed purchases never fire on GCP/Azure #712.
• internal/config/store_postgres.go (CleanupOldExecutions doc comment) and store_postgres_retention_integration_test.go: comments only.
• .github/workflows/ci.yml (shard-timing comment) and terraform/environments/aws/ci-cd-permissions/policy_boundary.tf: comments only. The role list in the boundary comment no longer names the cleanup Lambda role. It now says five EventBridge invoker roles, which matches the module tree, so the total stays twelve.
• CHANGELOG.md: added an entry under Unreleased "Removed".

Added:

• internal/config/purchase_executions_delete_guard_test.go: TestPurchaseExecutionsDeletedOnlyByConfigStore fails if any non-test Go file under internal/ or cmd/ outside internal/config has a string literal that deletes from or truncates purchase_executions. Retention can't drift into a second job again. TestPurchaseExecutionsDeletesMatcher pins what the matcher flags, so a broken matcher can't turn the guard into a silent pass. Like the existing TestNoUnscopedRecommendationsDelete, it checks one literal at a time.

Kept: everything in internal/ that the in-app task uses. CleanupOldExecutions is unchanged.

expires_at (column kept, no migration)

purchase_executions.expires_at and idx_purchase_executions_expires_at stay. Code that still reads it:

• CleanupOldExecutions branch 3 (expires_at IS NOT NULL AND expires_at < NOW() - retention, with the status allowlist)
• GetPendingExecutions (AND (expires_at IS NULL OR expires_at > NOW()))
• the row scanner (exec.TTL = ttlFromTime(expiresAt.Time)) and every execution SELECT column list
• written by SavePurchaseExecution from PurchaseExecution.TTL, which no production path sets

Legacy or imported rows can still carry non-NULL values, which is why these readers stay. Dropping the column or the TTL field would be a separate decision with its own migration.

Terraform: what the next apply destroys

No environment root in this repo instantiates any of the three modules, so applying terraform/environments/{aws,gcp,azure} destroys nothing and plans are unchanged. Only an out-of-tree root that sourced one of these module paths directly would lose resources on its next init/apply. For completeness:

• AWS compute/aws/cleanup-lambda: aws_iam_role.cleanup (<stack>-cleanup-lambda), aws_iam_role_policy_attachment.cleanup_basic, aws_iam_role_policy_attachment.cleanup_vpc, aws_iam_role_policy.cleanup_secrets, aws_cloudwatch_log_group.cleanup (/aws/lambda/<stack>-cleanup), aws_lambda_function.cleanup, aws_cloudwatch_event_rule.cleanup_schedule, aws_cloudwatch_event_target.cleanup, aws_lambda_permission.cleanup_eventbridge, aws_cloudwatch_metric_alarm.cleanup_errors
• GCP compute/gcp/cleanup-function: google_service_account.cleanup, google_secret_manager_secret_iam_member.cleanup_db_password, google_cloudfunctions2_function.cleanup, google_storage_bucket.function_source (force_destroy = false, so a destroy fails until the bucket is emptied), google_cloud_scheduler_job.cleanup, google_cloudfunctions2_function_iam_member.cleanup_invoker
• Azure compute/azure/cleanup-function: azurerm_storage_account.cleanup, azurerm_service_plan.cleanup, azurerm_user_assigned_identity.cleanup, azurerm_role_assignment.cleanup_kv_secrets_user, azurerm_linux_function_app.cleanup, azurerm_logic_app_workflow.cleanup_trigger, azurerm_logic_app_trigger_recurrence.cleanup, azurerm_logic_app_action_http.cleanup

Closes #357

Verification

All of the following is local unit, guard-script and Terraform-static evidence. Nothing was planned, applied or deployed.

Fail-before (guard test added on an untouched tree, before any deletion):

$ go test ./internal/config/ -count=1 -run 'TestPurchaseExecutionsDeletedOnlyByConfigStore|TestPurchaseExecutionsDeletesMatcher' -v
--- FAIL: TestPurchaseExecutionsDeletedOnlyByConfigStore
    .../cmd/cleanup-lambda/main.go:126:26: deletes from purchase_executions outside internal/config; ...
--- PASS: TestPurchaseExecutionsDeletesMatcher

Pass-after (same command after the deletion): both PASS, as does TestNoUnscopedRecommendationsDelete.

Mutation probe: with the internal/config skip removed from the walk, the guard flags internal/config/store_postgres.go:2391 (the real multi-line CleanupOldExecutions DELETE). That shows the scanner sees real store SQL and that the skip is what scopes it. The probe was reverted.

Other commands (with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true):

• go build ./..., go vet ./..., go vet -tags integration ./...: all OK
• go mod tidy -diff: no diff (aws-lambda-go is still used by cmd/lambda)
• go test -count=1 on ./internal/config/, ./terraform/modules/compute/{aws,gcp,azure}/, ./terraform/environments/aws/ci-cd-permissions/ (boundary/policy guards that walk every AWS module) and ./internal/mocks/: all ok
• go test -tags integration -count=1 -run 'TestRampRetention_OldCleanupWritersPreserveHeldRows' ./internal/config/ (only its comment changed): PASS on real Postgres (testcontainers)
• golangci-lint run --allow-parallel-runners ./internal/config/ with and without --build-tags integration: 0 issues
• gocyclo -over 10 -ignore "_test\.go" internal/config/store_postgres.go: no findings (comment-only change)
• shellcheck on the four touched scripts: clean. bash scripts/test-azure-kv-access-policy.sh: 25 passed, 0 failed. bash scripts/test-gcp-secret-scope.sh: 59 passed, 0 failed. Both check-*.sh exit 0 on the default scan roots.
• terraform fmt -check plus terraform init -backend=false and terraform validate on terraform/environments/aws/ci-cd-permissions (comment change): valid. terraform fmt -check -recursive terraform/modules/compute: clean.

Not run locally: actionlint (not installed; the ci.yml change is a YAML comment) and trivy config (not installed). The three removed .trivyignore IDs have no remaining source resource by git grep, but the pre-commit Trivy hook is the real check.

Combined-tree verification. Tested on a local, never-pushed tree: queue/platform @ 594ac3ec = origin/main ae6854b3 plus:

Branch	Tested head
fix/issue-357-delete-cleanup-lambda	56989d8
fix/issue-36-archera-gate	2d22986
fix/issue-35-assert-api-a	f793689
fix/issue-35-assert-api-b	fac72f3
fix/issue-120-stale-rec-sweep	07c2e9c
fix/issue-981-summary-sql	7056746

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.

• 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 (-tags integration -count=1, one package at a time): internal/api, internal/auth, internal/config, internal/scheduler, internal/server and internal/testutil pass.
• Secrets: git-secrets on the changed files found nothing, and a planted fake key was flagged; scripts/test-git-secrets-allowlist.sh passes.

Out of scope

• No DB migration: expires_at, its index and PurchaseExecution.TTL stay (see above).
• CleanupOldExecutions is unchanged.
• docs/audits/codebase-audit-2026-09-02.md still mentions the modules. It is a verbatim historical audit record and is left as is.
• The issue's LOW finding (RATE_LIMIT_DOMAIN LIKE wildcards) was already handled by fix(clear-rate-limit): match RATE_LIMIT_DOMAIN literally in the LIKE pattern #1272, and the terminal-status guard landed in fix(cleanup): only delete settled executions when their TTL lapses #1173. This PR finishes the remaining MEDIUM item.

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

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• 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 56989d88a818435c4eb3985d2e3aef735db7695d

Blocking:

• none

Non-blocking:

• .trivyignore: dropping AVD-AZU-0004, AVD-AZU-0012 and GCP-0001 looks right. A grep of terraform/, iac/, arm/ and cloudformation/ finds no remaining azurerm_*function_app, azurerm_storage_account or google_storage_bucket. I could not run trivy config either (it isn't installed here), so the pre-commit Trivy hook is the only real check. Watch that job before merging.
• Out-of-tree consumers: the body is right that no environment root here sources the three modules, so in-repo plans don't change. An external root that pins these paths would fail at init and would then have to destroy them. The GCP source bucket has force_destroy = false, so it has to be emptied by hand. The body already says this; the release notes could repeat it.
• known_issues/35_legacy_guard_followups.md: the "Resolved by" paragraph still points at cmd/cleanup-lambda/main_integration_test.go as the coverage. The new "Update (chore(pkg/cmd): medium/low findings from the 2026-07-28 full review [platform part] #357)" note above it explains the file is gone, so this is just stale prose and is acceptable.
• purchase_executions_delete_guard_test.go finds the repo root through runtime.Caller, so it would break under go test -trimpath. TestNoUnscopedRecommendationsDelete already has the same dependency, so this is not a new risk.

Attacked:

• Grepped the whole tree (except docs/audits) for cleanup-lambda, cleanup_lambda, cleanup-function, cleanupExpiredRecords and CleanupEvent. Only comments, changelog and known_issues text remain. No Dockerfile, Makefile, workflow, CI-CD permission or script still refers to the deleted modules or binary.
• The CI terraform-test loops glob compute/*/*/tests, and the deleted modules had no tests/ directory.
• Counted the non-test aws_iam_role resources under terraform/modules: there are 12, five of them EventBridge invokers, which matches the rewritten boundary comment. On main there were 13 including the cleanup role, so the old comment was off by one and the new one is correct.
• Fail-before: I restored cmd/cleanup-lambda/main.go from main into the scratch tree. TestPurchaseExecutionsDeletedOnlyByConfigStore then fails at main.go:126:26, and it passes again once the file is removed. The matcher test pins the positive and negative samples.
• Ran go build ./..., go vet ./..., go vet -tags integration ./... and go mod tidy -diff (no diff). Ran go test -race -short -count=1 on ./internal/config/, ./terraform/modules/compute/... and ./terraform/environments/aws/ci-cd-permissions/: all ok. scripts/test-azure-kv-access-policy.sh gives 25/0 and scripts/test-gcp-secret-scope.sh gives 59/0.
• Ran go test -race -tags integration on TestRampRetention_OldCleanupWritersPreserveHeldRows plus both guard tests, against real Postgres: PASS. I also started golangci-lint on internal/config, but it was killed at the background time limit because the shared VM was overloaded, so I have no lint result. The author's 0-issues claim and the CI lint job are what stand.
• Against issue chore(pkg/cmd): medium/low findings from the 2026-07-28 full review [platform part] #357: the MEDIUM item is resolved by deleting the job, which the acting owner decided on 2026-10-29. The guard test keeps any other purchase_executions deleter out. The LOW item was already fixed by fix(clear-rate-limit): match RATE_LIMIT_DOMAIN literally in the LIKE pattern #1272. Closes #357 is correct.

Body-stale:

• none

_{Reviewer: rQ1. 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]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 56989d88a818435c4eb3985d2e3aef735db7695d

Blocking:

• none

Non-blocking:

• terraform/environments/gcp/ci-cd-permissions/service_account.tf:18 (roles/cloudfunctions.developer) and terraform/environments/azure/ci-cd-permissions/locals_compute.tf:9-10 (Microsoft.Web/serverFarms/*, Microsoft.Web/sites/*): after this PR no resource in the tree is a Cloud Function, Function App or App Service plan (git grep 'resource "google_cloudfunctions' and the azurerm *function_app* / service_plan / *web_app* types return nothing), so these deploy-role grants look dead. A least-privilege trim belongs in a follow-up. It changes nothing until the ci-cd-permissions roots are applied, and someone should confirm no out-of-tree root still needs them.
• The guard walks only internal/ and cmd/. That is enough for production code today, and the doc comment says it is best-effort (one literal at a time).
• (Also raised by an earlier review, and I agree:) the "Resolved by" paragraph in known_issues/35_legacy_guard_followups.md still names cmd/cleanup-lambda/main_integration_test.go as its coverage. The new Update note above it covers this, so the paragraph is stale prose only.

Attacked:

• Deletion completeness: git grep -i 'cleanup-lambda|cleanup-function|cleanup_function|cleanupExpiredRecords' (outside docs/audits and CHANGELOG) finds only the updated comments. Nothing in the Dockerfile, Makefiles, workflows, dependabot.yml or any environment root sourced the modules, on main or on this head. go mod tidy -diff is clean (aws-lambda-go is still used by cmd/lambda). The policy_boundary count is correct: 12 aws_iam_role resources remain, 5 of them EventBridge invoker roles. Removing the .trivyignore entries is safe: no azurerm_*function_app*, azurerm_storage_account or google_storage_bucket resource remains, and the only ARM Microsoft.Storage hit is a role-assignment fixture.
• Fail-before: I restored cmd/cleanup-lambda/main.go from origin/main in the scratch tree. TestPurchaseExecutionsDeletedOnlyByConfigStore then fails at cmd/cleanup-lambda/main.go:126:26. It passes again with the file removed.
• Ran: go build ./..., go vet ./..., go vet -tags integration ./...; go test -race -short -count=1 on ./internal/config/ ./terraform/modules/compute/... ./terraform/environments/aws/ci-cd-permissions/ (all ok); go test -race -tags integration -run 'TestRampRetention|TestCleanupOldExecutions' ./internal/config/ (ok); scripts/test-azure-kv-access-policy.sh (25/0) and scripts/test-gcp-secret-scope.sh (59/0). golangci-lint on ./internal/config/ was not completed locally: it ran 30 minutes under load from other agents on the shared VM and was stopped. The only Go change there is a new _test.go file plus comments, and CI lint covers it.
• The acting-owner decision on chore(pkg/cmd): medium/low findings from the 2026-07-28 full review [platform part] #357 (delete) is recorded on the issue. The LOW item was already fixed by fix(clear-rate-limit): match RATE_LIMIT_DOMAIN literally in the LIKE pattern #1272, so Closes #357 is right.

Body-stale: none

_{Reviewer: rQ2. 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 56989d88a818435c4eb3985d2e3aef735db7695d merged onto main (d71af683), together with #1591, #1595, #1596 in that order (combined tree 71d6c3d4, 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 2 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: 2 approving independent review(s) on 56989d88 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 56989d88a818435c4eb3985d2e3aef735db7695d.