feat/energia-sweep
needs attention9035c81 · incrementalPR #331reviewed 2026-07-22 00:13 UTC0H · 1M · 2L · 1I- Purpose
- Provide the automatic nightly trigger that fills `utility_contract_findings` (the energia anomaly ledger landed in #328) so the read API starts seeing rows without any manual invocation.
- Goal
- Nightly-sweep Lambda (`findings-sweep`) that enumerates entitled orgs, runs the idempotent `detectFindingsForOrgShell` per org, isolates per-org failures, and emits CloudWatch EMF metrics + structured logs.
- Sub-goals
- SG-1–13: 12 device-integration manifests, catalog seeds, ARN registry, CDK paths, migration scripts, 1M calendar-month granularity, derivation engine coexistence, Loop-C review remediations
- This commit (SG-test): DB-injectable `runSweep(db, actor)` extraction + full DB integration test suite covering entitlement gate, idempotency, outbox, and failure isolation
- What
- Extracted `runSweep(db, actor)` from the argless `handler()` so the imperative shell body is testable with any injected `Database`. Added `sweep.integration.test.ts` (577 lines) covering 5 scenarios against real DB transactions. Added `setup.ts` (dotenv `.env.local` loader + `hasTestDatabaseUrl` soft gate). Updated `vitest.config.ts` with path aliases for workspace packages, `setupFiles`, and increased timeouts for DB-backed hooks.
- Why
- The `handler()` entry point was untestable (it closed over the module-level `database` singleton and a fixed system actor). Extracting `runSweep` is the minimal dependency-injection seam needed to drive the sweep against a real transactional DB in tests, following the pattern of the cross-domain harness.
- Areas
- services/findings/src/__tests__+611−0services/findings/src/handlers/findings-sweep.handler.ts+43−9services/findings/vitest.config.ts+19−0services/findings/package.json+1−0pnpm-lock.yaml+3−0
- Blast
- 674 lines added, 9 removed. Scope is entirely within `services/findings/` (tests + one handler refactor) and the lockfile. No API, schema, or CDK surface changes.
Findings · 4
correctness1
`.env.local` path traversal depth is correct but undocumented
services/findings/src/__tests__/setup.ts:21
`resolve(__dirname, '../../../../.env.local')` navigates from `services/findings/src/__tests__/` four levels up to the repo root. This is correct and mirrors the cross-domain harness — just worth verifying on any future directory restructure. A comment like `// repo root` would make the intent auditable at a glance, following the pattern in `domains/cross-domain/src/__tests__/setup.ts`.
conventions1
`db` parameter in `runSweep` shadows the module-level `db` import
services/findings/src/handlers/findings-sweep.handler.ts:75
`runSweep(db: Database, actor)` — the parameter `db` shadows `import { db } from '@batu/database'` at the module level. TypeScript resolves the parameter correctly within the function body, so the behaviour is right, but the naming collision is confusing: a reader inside `runSweep` cannot tell whether `fetchAllOrganizations(db)` uses the injected parameter or the module singleton. `sweepOneOrg` and `fetchAllOrganizations` already use `dbClient` / `db` (parameter) consistently — renaming the `runSweep` parameter to `database` or `dbClient` removes the ambiguity at zero cost.
tests2
`globalCountAfter1 === globalCountAfter2` comparison is fragile in a shared DB
services/findings/src/__tests__/sweep.integration.test.ts:491
`totalFindingCount()` queries ALL findings with no org-namespace filter. If another process (or a concurrent test run) writes a finding between the two `runSweep` calls the assertion fails spuriously. The contract-scoped `aCountAfter2 === aCountAfter1` assertion on the same line already proves idempotency without the cross-test coupling. The global comparison adds noise without extra signal — consider dropping it or narrowing it to the test's own contract IDs.
Failure-isolation test (block 5) is DB-gated but uses no DB
services/findings/src/__tests__/sweep.integration.test.ts:522
The `aggregateSweep` fold test in block 5 is wrapped in `describeDb` (skipped when `POSTGRES_URL` is absent), but it is a pure in-memory unit test that does not touch the database. This means it won't run in any environment that lacks a DB connection (e.g., basic CI without Supabase wiring), silently hiding coverage. `aggregateSweep` is already exercised in `sweep.test.ts`, so this is low risk, but the misclassification is worth noting.