Problem
Report 04 (server/config) and report 06 (analytics) have several medium/low/nit findings in `internal/server/` and `cmd/server/main.go` that were not included in the initial PR #1040 commits. This issue tracks the remaining FOLD-1040 bucket.
04-M3 — `migrationsTimeout`/`runMigrations` package globals are not parallel-safe
`internal/server/app.go:154,172`: Both are package-level vars overwritten by tests. Tests that modify them must not call `t.Parallel()`, a constraint the compiler cannot enforce. Moving them to struct fields (defaulted in the constructor) makes each Application instance self-contained and test-parallel-safe.
04-M4 — `resolveScheduledTaskSecret` errors are swallowed in bearer mode, surfacing as a misleading downstream error
`app.go:273-283,298-312,321`: When the SecretResolver lookup fails in bearer mode, the code falls back to the plaintext env var (normally empty in prod) and `buildScheduledAuth` then fails with "bearer mode requires SCHEDULED_TASK_SECRET" — pointing the operator at the wrong env var. The root-cause secret-resolution error is buried in the log. Fix: return `(string, error)` from `resolveScheduledTaskSecret` and propagate the error in bearer mode.
04-M5 — `determineRuntimeMode` reads `AWS_LAMBDA_RUNTIME_API` directly
`cmd/server/main.go:110`: Reads the env var the `runtime.IsLambda()` helper was introduced to encapsulate. A future change to detection logic silently misses this call site.
04-M6 — Two divergent static-file containment checks (Lambda vs HTTP)
`static.go:25-56` vs `static.go:77-111`: `spaHandler.ServeHTTP` (HTTP path) uses no explicit abs-prefix check; `resolveStaticFilePath` (Lambda path) uses `HasPrefix(absFile, absDir)` which is subtly weak (no trailing separator). Extract one correct shared helper with separator-aware prefix and use it from both paths.
04-L2 — `hasFileContent` is dead code
`static.go:189-197`: No production caller; misleading comment claims startup-validation use that never happens.
04-L4 — Health check silently discards `json.Encode` error
`health.go:66`: `json.NewEncoder(w).Encode(health)` return value is ignored. `handleScheduledHTTP` logs it; inconsistent. Log for parity.
04-L5 — Scheduled handler re-parses task type with manual string split
`http.go:143-152`: `parts[2]` re-derives the segment the mux already isolated. Use `strings.TrimPrefix` or `r.PathValue` instead.
04-N1 — Version threaded via env round-trip
`cmd/server/main.go:33`, `cmd/lambda/main.go:48`: Both push `Version` into `os.Setenv("VERSION",...)` so `LoadApplicationConfig` can read it back. Awkward; pass directly via `ApplicationConfig`.
04-N3 — `isLambdaRuntime()` wrapper has a "do not use" comment but is still called
`app.go:138-140,263,385`: Either inline `runtime.IsLambda()` at call sites or drop the deprecation note.
04-N4 — Unknown Lambda event defaults to scheduled without a distinct error
`lambda.go:43-45`: Unrecognised payload formats are silently treated as scheduled events, masking the real "unknown event shape" cause.
06-M4 — `REFRESH MATERIALIZED VIEW CONCURRENTLY` inside a migration transaction
`migrations/000003:162-177`, `server/handler.go:211`: PostgreSQL rejects `REFRESH MATERIALIZED VIEW CONCURRENTLY` inside a transaction block. The initial migration calls `refresh_savings_materialized_views()` which uses CONCURRENTLY — this will fail at first run. Fix: use plain (non-concurrent) refresh inside the migration (views are empty anyway); reserve CONCURRENTLY for the runtime scheduled-task path. Also improve error visibility at handler.go:211 (currently logged as Warning only).
Evidence
Source: code-review report 04 (server/config) findings M3-M6, L2, L4-L5, N1-N4 and report 06 finding M4. All files are in the `fix/server-transport-config` branch that PR #1040 modifies.
Fix
Implement as additional commits on PR #1040 branch. Regression tests for M3 (parallel-safe migration fields), M4 (bearer-mode error propagation), M5 (IsLambda call), and 06-M4 (migration transaction guard).
Closes after PR #1040 merges.
Problem
Report 04 (server/config) and report 06 (analytics) have several medium/low/nit findings in `internal/server/` and `cmd/server/main.go` that were not included in the initial PR #1040 commits. This issue tracks the remaining FOLD-1040 bucket.
04-M3 — `migrationsTimeout`/`runMigrations` package globals are not parallel-safe
`internal/server/app.go:154,172`: Both are package-level vars overwritten by tests. Tests that modify them must not call `t.Parallel()`, a constraint the compiler cannot enforce. Moving them to struct fields (defaulted in the constructor) makes each Application instance self-contained and test-parallel-safe.
04-M4 — `resolveScheduledTaskSecret` errors are swallowed in bearer mode, surfacing as a misleading downstream error
`app.go:273-283,298-312,321`: When the SecretResolver lookup fails in bearer mode, the code falls back to the plaintext env var (normally empty in prod) and `buildScheduledAuth` then fails with "bearer mode requires SCHEDULED_TASK_SECRET" — pointing the operator at the wrong env var. The root-cause secret-resolution error is buried in the log. Fix: return `(string, error)` from `resolveScheduledTaskSecret` and propagate the error in bearer mode.
04-M5 — `determineRuntimeMode` reads `AWS_LAMBDA_RUNTIME_API` directly
`cmd/server/main.go:110`: Reads the env var the `runtime.IsLambda()` helper was introduced to encapsulate. A future change to detection logic silently misses this call site.
04-M6 — Two divergent static-file containment checks (Lambda vs HTTP)
`static.go:25-56` vs `static.go:77-111`: `spaHandler.ServeHTTP` (HTTP path) uses no explicit abs-prefix check; `resolveStaticFilePath` (Lambda path) uses `HasPrefix(absFile, absDir)` which is subtly weak (no trailing separator). Extract one correct shared helper with separator-aware prefix and use it from both paths.
04-L2 — `hasFileContent` is dead code
`static.go:189-197`: No production caller; misleading comment claims startup-validation use that never happens.
04-L4 — Health check silently discards `json.Encode` error
`health.go:66`: `json.NewEncoder(w).Encode(health)` return value is ignored. `handleScheduledHTTP` logs it; inconsistent. Log for parity.
04-L5 — Scheduled handler re-parses task type with manual string split
`http.go:143-152`: `parts[2]` re-derives the segment the mux already isolated. Use `strings.TrimPrefix` or `r.PathValue` instead.
04-N1 — Version threaded via env round-trip
`cmd/server/main.go:33`, `cmd/lambda/main.go:48`: Both push `Version` into `os.Setenv("VERSION",...)` so `LoadApplicationConfig` can read it back. Awkward; pass directly via `ApplicationConfig`.
04-N3 — `isLambdaRuntime()` wrapper has a "do not use" comment but is still called
`app.go:138-140,263,385`: Either inline `runtime.IsLambda()` at call sites or drop the deprecation note.
04-N4 — Unknown Lambda event defaults to scheduled without a distinct error
`lambda.go:43-45`: Unrecognised payload formats are silently treated as scheduled events, masking the real "unknown event shape" cause.
06-M4 — `REFRESH MATERIALIZED VIEW CONCURRENTLY` inside a migration transaction
`migrations/000003:162-177`, `server/handler.go:211`: PostgreSQL rejects `REFRESH MATERIALIZED VIEW CONCURRENTLY` inside a transaction block. The initial migration calls `refresh_savings_materialized_views()` which uses CONCURRENTLY — this will fail at first run. Fix: use plain (non-concurrent) refresh inside the migration (views are empty anyway); reserve CONCURRENTLY for the runtime scheduled-task path. Also improve error visibility at handler.go:211 (currently logged as Warning only).
Evidence
Source: code-review report 04 (server/config) findings M3-M6, L2, L4-L5, N1-N4 and report 06 finding M4. All files are in the `fix/server-transport-config` branch that PR #1040 modifies.
Fix
Implement as additional commits on PR #1040 branch. Regression tests for M3 (parallel-safe migration fields), M4 (bearer-mode error propagation), M5 (IsLambda call), and 06-M4 (migration transaction guard).
Closes after PR #1040 merges.