[IAmJSD]

Root cause

One Application held two answers to "am I on Lambda". LoadApplicationConfig captures cfg.IsLambda from runtime.IsLambda() once, and the post-DB scheduler in reinitializeAfterConnect uses app.appConfig.IsLambda. But NewApplicationFromDeps built the pre-DB scheduler with a live runtime.IsLambda() (internal/server/app.go, NewScheduler call), ignoring the cfg it was handed. A caller passing ApplicationConfig{IsLambda: true} without AWS_LAMBDA_RUNTIME_API got a container-mode scheduler until ensureDB succeeded, and for good if the DB stayed unreachable. In that window ListRecommendations could spawn the detached 5-minute background refresh (maybeKickBackgroundRefresh) inside a Lambda invocation, which is exactly what the scheduler's isLambda gate exists to prevent.

The API handler had the same split. getRIUtilizationCache (SWR refresh goroutine gate) and expireStaleExecutions (async vs sync stale sweep) each read runtime.IsLambda() live instead of the Application's value.

Fix

ApplicationConfig.IsLambda is now the only source of truth inside an Application, threaded to every consumer that branches on Lambda mode:

Consumer	Before	After
Pre-DB scheduler (NewApplicationFromDeps)	runtime.IsLambda() (live)	cfg.IsLambda
Post-DB scheduler (reinitializeAfterConnect)	app.appConfig.IsLambda	unchanged
OIDC issuer prime from Lambda (NewApplicationFromDeps)	cfg.IsLambda	unchanged
API handler, RI utilization SWR gate	runtime.IsLambda() (live)	h.isLambda, from new HandlerConfig.IsLambda
API handler, stale-execution sweep	runtime.IsLambda() (live)	h.isLambda
Pre-DB and post-DB api.NewHandler calls	(no field)	pass cfg.IsLambda / app.appConfig.IsLambda

Rate limiter: on current main it no longer branches on IsLambda (temporary in-memory limiter pre-DB on every runtime, DB-backed after ensureDB), so nothing changes there.

Scheduler.IsLambda() and Handler.IsLambda() are new read-only accessors so the server package can check its wiring. They return the constructed value and never re-read the environment. Lambda detection itself (internal/runtime.IsLambda) is unchanged.

internal/ and cmd/ now contain no runtime.IsLambda() calls apart from LoadApplicationConfig (the single capture) and cmd/server/main.go's determineRuntimeMode (entry-point mode selection, before any config exists).

Closes #110

Tests

• internal/server/app_islambda_test.go (unit): NewApplicationFromDeps with IsLambda: true while all Lambda env vars (AWS_LAMBDA_RUNTIME_API, AWS_LAMBDA_FUNCTION_NAME, AWS_EXECUTION_ENV, LAMBDA_TASK_ROOT) are unset, and with IsLambda: false while they are set. Asserts the pre-DB scheduler and API handler both follow the config.
• internal/server/app_islambda_integration_test.go (integration tag, Postgres testcontainer): same two directions after ensureDB, covering the post-DB scheduler and handler rebuilt in reinitializeAfterConnect.
• internal/api/ri_utilization_cache_test.go: NewHandler(HandlerConfig{IsLambda: x}) with the env set to the opposite. Asserts Handler.IsLambda() and the RI cache's isLambda follow the config.
• internal/api/handler_history_test.go (ExpireIfStale_LambdaGuard) and handler_history_actionable_starvation_test.go: these used to flip AWS_LAMBDA_RUNTIME_API. They now set Handler.isLambda, and the env is set to the opposite value to show it is ignored.
• The internal/scheduler isLambda guard tests from test(scheduler): make the Lambda no-refresh tests bite again #971/test(scheduler): make the LambdaGuard control catch a non-blocking harness (#971 follow-ups) #988 pass unchanged.

Verification (local, macOS, at e4d9fb7)

Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null. No real cloud calls: a configured issuer URL skips the Lambda function-URL lookup, and the integration test uses static dummy keys against a local container.

• go build ./... and go vet ./...: clean
• go test -count=1 on ./internal/server/..., ./internal/scheduler/..., ./internal/mocks/..., ./internal/api/...: all ok
• go test -tags integration ./internal/server/ -run 'TestApplicationRateLimiter|TestEnsureDB_IsLambda': ok
• golangci-lint v2.10.1 run --allow-parallel-runners ./...: 0 issues
• gocyclo -over 10 on every changed non-test .go file: no findings
• git-secrets 1.3.0 (--register-aws, isolated HOME, temp copy) --scan on changed files: clean. A planted AKIA probe was flagged, so the scanner was live. bash scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL
• Fail-before and mutation probes. Each mutation was applied on its own and the matching test was checked to fail:
  • M1: pre-DB scheduler back to runtime.IsLambda() (the original bug). Killed by the unit test, both directions.
  • M2: pre-DB HandlerConfig.IsLambda set to the live env value. Killed.
  • M3/M4: post-DB handler or scheduler set to the live env value. Killed by the integration test.
  • M5/M6: NewHandler or getRIUtilizationCache reading the live env. Killed.
  • M7: expireStaleExecutions reading the live env. Killed: the Lambda sub-test sees no synchronous sweep, and the non-Lambda sub-test blocks.

Out of scope

• cmd/server/main.go determineRuntimeMode still auto-detects Lambda via runtime.IsLambda(). If someone forces --mode=lambda outside a Lambda environment, LoadApplicationConfig still captures IsLambda=false from the env. That is entry-point mode selection, not this in-Application divergence. Left alone; it may be worth a follow-up.
• oidc.PrimeIssuerURLFromLambda reads AWS_LAMBDA_FUNCTION_NAME to name the function to look up. It is already gated by cfg.IsLambda and does not decide the mode.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Lambda-specific background processing now consistently follows the application’s configured mode, even when environment settings indicate otherwise.
  • Scheduler and API behavior now use the same configured mode throughout application startup and database initialization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: f2bfca7c-657c-4ab6-894d-02d457ff5a0b

📥 Commits

Reviewing files that changed from the base of the PR and between 8c93e39 and 053b137.

📒 Files selected for processing: 10

• internal/api/handler.go
• internal/api/handler_history.go
• internal/api/handler_history_actionable_starvation_test.go
• internal/api/handler_history_test.go
• internal/api/ri_utilization_cache_test.go
• internal/api/types.go
• internal/scheduler/scheduler.go
• internal/server/app.go
• internal/server/app_islambda_integration_test.go
• internal/server/app_islambda_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The application now passes its configured Lambda mode to schedulers and API handlers. These components retain that value instead of relying on runtime environment detection for RI cache setup and history expiration sweeps.

Changes: Configured Lambda mode

Layer / File(s)	Summary
Capture Lambda mode in handlers and schedulers internal/api/types.go, internal/api/handler.go, internal/scheduler/scheduler.go, internal/api/ri_utilization_cache_test.go	HandlerConfig adds IsLambda. The handler stores this value, uses it to configure the RI utilization cache, and exposes it through Handler.IsLambda(). Scheduler.IsLambda() exposes the scheduler's stored mode. Tests check that the handler and cache follow the configured value when the environment differs.
Pass configured mode through application setup internal/server/app.go, internal/server/app_islambda_test.go, internal/server/app_islambda_integration_test.go	Application setup passes the configured mode to initial and post-database scheduler and API handler construction. Tests check that the wiring follows the configured mode before and after database setup.
Use captured mode for history sweeps internal/api/handler_history.go, internal/api/handler_history_actionable_starvation_test.go, internal/api/handler_history_test.go	The history expiration sweep selects synchronous handling from the handler's captured mode. Tests set the handler mode directly and verify behavior when the environment indicates the opposite mode.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to 053b1

This change makes the Lambda mode consistent across the scheduler and API handlers instead of rechecking the environment. No concrete merge-blocking risk was found, and tests cover the disagreeing config and environment cases.

Security architecture risk: 🔵 Low · up to 053b1

The change consistently propagates one runtime-mode value without introducing a request-controlled selector or an identified expansion of privileges. Initialization and cleanup safeguards remain intact. Risk is low, with callers now responsible for supplying the correct mode.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — History cleanup operates on stale executions collected before response account filtering, so maintenance scope is not limited to rows visible in the response. This ordering predates the PR; the changed mode gate selects execution timing rather than different execution identities.

Trust Boundaries and Controls

• inferred — Changing the mode requires control over process configuration or application construction in the inspected flow. The new accessors only report captured state; they do not provide callers with a mutation or privilege-granting operation.

Resilience and Maintainability Implications

• observed — Expiration remains bounded by DefaultListLimit. Each status change and suppression deletion share a transaction, with rollback on error and a database status predicate preventing expiration of an execution that has already left pending or notified. These safeguards are unchanged by the PR and contain partial failure and concurrent sweeps.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main change: passing one configured IsLambda value through all Application consumers.
Linked Issues check		The PR meets the coding requirements in issue #110. NewApplicationFromDeps passes cfg.IsLambda to the pre-DB scheduler and API handler. The post-DB wiring passes app.appConfig.IsLambda. The hand…
Out of Scope Changes check		The changes remain connected to issue #110. Handler configuration, scheduler and handler accessors, and the API tests support propagation and verification of one captured Lambda value. The stale-sweep…

Full details: Linked Issues check

Explanation

The PR meets the coding requirements in issue #110. NewApplicationFromDeps passes cfg.IsLambda to the pre-DB scheduler and API handler. The post-DB wiring passes app.appConfig.IsLambda. The handler uses h.isLambda for the RI cache refresh gate and stale-execution sweep. The new unit and integration tests cover both configured modes with opposite or absent Lambda environment variables. LoadApplicationConfig remains the configuration capture point, and the entry-point runtime check remains outside this issue's scope.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to issue #110. Handler configuration, scheduler and handler accessors, and the API tests support propagation and verification of one captured Lambda value. The stale-sweep and RI-cache changes remove live mode reads within the application's constructor reach. No unrelated implementation change is demonstrated by the supplied change summary or reviewed file evidence.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[cristim]

Independent review of e4d9fb7: draft: blocks ready

No code defects found. The only blockers to ready are gate items that are still open (CI not finished, no PR-side verification record from an independent party until this comment).

Findings

None BLOCKING on correctness or security.

NON-BLOCKING (follow-up, already acknowledged in the PR body):

• cmd/server/main.go:117 and internal/server/app.go:317. --mode=lambda or RUNTIME_MODE=lambda outside a real Lambda env still yields cfg.IsLambda=false from LoadApplicationConfig, so mode selection and the in-Application value can differ. Pre-existing and not the divergence #110 describes. Fix idea: derive IsLambda from the resolved runtime mode.

What I attacked

• Single source of truth. Grepped cmd/ and internal/ non-test code for IsLambda, isLambda, AWS_LAMBDA_*, AWS_EXECUTION_ENV, LAMBDA_TASK_ROOT, runtime.IsLambda. Remaining live reads: internal/runtime/runtime.go:18 (definition), internal/server/app.go:317 (the one capture), cmd/server/main.go:117 (entry-point mode choice, before any config), internal/oidc/lambda_issuer.go:33 (function name only, gated by cfg.IsLambda at app.go:466). Detection is only AWS_LAMBDA_RUNTIME_API non-empty, so Cloud Run, Container Apps, Fargate and local server runs are never classified as Lambda (they leave it unset), and nothing was added that changes that.
• Constructor coverage. scheduler.NewScheduler and api.NewHandler have exactly three production call sites in app.go (pre-DB scheduler :489, pre-DB handler :522, post-DB scheduler :854, post-DB handler :885). All now use cfg.IsLambda or app.appConfig.IsLambda. No other HandlerConfig{} / SchedulerConfig{} construction in non-test code.
• Consumers: scheduler.go:1191, 1460, 1510 (background refresh, cold-start kick) read s.isLambda; api ri_utilization_cache.go:121 reads the cache field fed from h.isLambda (handler.go:157); handler_history.go:269 reads h.isLambda. Non-Lambda behavior is unchanged: with the env unset and cfg.IsLambda=false the values are identical to the old live reads. getRIUtilizationCache is lazy via sync.Once, so the field is set before first use.
• Mutation probes (reverted, worktree clean afterwards): (1) pre-DB scheduler back to runtime.IsLambda(): TestNewApplicationFromDeps_IsLambdaSingleSourceOfTruth fails (expected false). (2) expireStaleExecutions back to runtime.IsLambda(): ExpireIfStale_LambdaGuard non-Lambda subtest fails (blocks on sweep). Both the original bug and a regression of the handler gate are caught.
• Tests at this SHA (AWS config files nulled): go build ./... ok; go test -count=1 ./internal/server/... ./internal/scheduler/... ./internal/api/... 4349 passed. go test -tags integration ./internal/server -run TestEnsureDB_IsLambda 3 passed (testcontainers Postgres, cleaned up).
• Integration file build tag is integration; the CI integration-tests job (ci.yml:346) runs -tags=integration, and make test-compile-integration vet-compiles it, so it is covered in CI.
• Scope/size: 250 additions, 10 files, under 400 added lines; no file over 500 lines introduced. No new helper duplicates an existing one (the two accessors are the minimal test seam).
• Labels: PR carries priority/p2, severity/medium, urgency/this-quarter, impact/few, effort/xs, type/bug, triaged; identical to issue #110.
• CodeRabbit: skipped (draft); no threads. Not pinged.

Gate status (origin/main Review gate)

• Independent adversarial review of the full diff naming the SHA: this comment.
• Actionable findings resolved: none open.
• CI green on SHA: NOT YET. Runs for this SHA were queued / checks pending at review time (CI - Build & Test, pre-commit). No action_required fork runs were present at review time; re-check when the draft is marked ready.
• Local verification on macOS exercising the real scenario with fail-before/pass-after: done above (my probes plus the author's M1-M7, which I partly reproduced; M3-M6 not re-run).
• Verdict, SHA and evidence recorded on the PR: this comment.

Blocks ready

1. PR is still a draft.
2. CI must finish green on e4d9fb7 (or a later SHA, which restarts this gate).

[cristim]

@IAmJSD this PR now conflicts with main in internal/api/handler_history.go (main gained history changes since e4d9fb7, e.g. #1038 and #1046's neighbours). The independent review at e4d9fb7 found no code defects and the gate is otherwise met. Please rebase onto main and push; if the range-diff of the PR's own commits is a pure rebase the verdict carries, otherwise the changed hunks get a delta review. The merge driver will merge once CI is green on the new head.

[IAmJSD]

Resolved in 053b137 by merging origin/main instead of rebasing, so the PR's own commits are unchanged and there was no force-push.

The only conflict was the import block of internal/api/handler_history.go. Main added internal/purchase (used by purchase.RedriveRefusalReason), and this PR had dropped internal/runtime because the file now reads h.isLambda. The resolution keeps purchase and leaves runtime out. Nothing else in the file conflicted.

Checked on the merged tree:

• The only non-test runtime.IsLambda() reads are still the two from the review: internal/server/app.go:317 (the single capture) and cmd/server/main.go:117 (entry-point mode choice).
• go build ./... and go vet on api, server and scheduler are clean.
• go test passes for ./internal/api/, ./internal/server/, ./internal/scheduler/ and ./internal/mocks/.
• golangci-lint reports 0 issues.

[cristim]

Merge driver check of the new head 053b137: it is a merge of main (8c93e39) into the reviewed e4d9fb7, and the PR's own commit is unchanged (git range-diff: 1: e4d9fb79 = 1: e4d9fb79). The only conflict resolution (git show --remerge-diff 053b137f) is the import block of internal/api/handler_history.go: it keeps main's new internal/purchase import and drops internal/runtime, which this PR stopped using. No other hunk differs from the automatic merge, so the independent review at e4d9fb7 carries to 053b137. Merging once CI is green on 053b137.

[cristim]

Gate record for 053b137 (origin/main CLAUDE.md "Review gate"):

• Independent review (fresh cold-brief agent, non-author) of e4d9fb7: no code defects (fix(server): thread one IsLambda value through every Application consumer #1027 (review)). Head 053b137 is a merge of main into e4d9fb7; the PR's own commit is unchanged (range-diff 1: e4d9fb79 = 1: e4d9fb79), and the only manual conflict resolution is the handler_history.go import block (keeps main's internal/purchase, drops internal/runtime, which this PR stopped using), as recorded in the merge-driver comment above. The verdict carries. Non-blocking note: --mode=lambda outside a real Lambda env still yields IsLambda=false (pre-existing; follow-up).
• CI on 053b137: latest CI - Build & Test and pre-commit runs (refreshed 2026-10-17T05:20Z against current main) completed/success; CI Success and Run pre-commit hooks pass.
• Local verification (by the reviewer): grep audit of every IsLambda/AWS_LAMBDA_* read in cmd/ and internal/, all three NewScheduler/NewHandler production call sites checked, package tests pass; non-Lambda behavior unchanged. No real Lambda run.
• CR skipped: quota-limited; covered by the independent review plus local verification.
• Labels mirrored from fix(server): two sources of truth for IsLambda in one Application can pin Lambda to the container path #110.
• Handed over from claude-cc-pr-merge-4 (its session could not merge).

Merging at 053b137 with --match-head-commit.