fix/tariff-cov

Stale-job reaper uses rounded minutes for threshold comparison instead of raw milliseconds

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts:106

In reapStaleActiveJob, ageMinutes is computed with Math.round(age_ms / 60_000), then the staleness check re-multiplies: ageMinutes * 60_000 < STALE_ACTIVE_JOB_MS. Because Math.round can round UP (e.g., 179.5 min rounds to 180), a job that is only ~179.5 minutes old would round to 180 and compare 10_800_000 < 10_800_000 = false, triggering a reap ~30 seconds early. Fix: compare raw milliseconds directly: if ((Date.now() - active.queuedAt.getTime()) < STALE_ACTIVE_JOB_MS).

reapStaleActiveJob does not re-read job status inside the transaction — stale-read TOCTOU

domains/utility/src/tariff-job/tariff-job.shells.ts:234

reapStaleActiveJob fetches the active job outside the transaction, then passes the already-resolved TariffJob object to failTariffJobShell. Inside the shell, resolveJob returns that object directly without re-reading from the DB within the transaction. tariffJobQueries.update has no WHERE status IN ('queued','running') guard, and decideFailJob is evaluated against the stale status. If the job transitions to completed between the initial find and the transaction, the update will overwrite a successful completion with failed. Fix: pass the job's publicId (not the object) to failTariffJobShell, letting resolveJob re-fetch within the transaction.

isTou check excludes genSpCost (semi-punta) from TOU detection

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:432

const isTou = componentsByZone.some((c) => [c.genBCost, c.genICost, c.genPCost].some(...)) checks only Base/Intermedia/Punta bands. genSpCost (semi-punta) is in ENERGY_FIELDS and accepted by hasEnergyComponent, but excluded from isTou. A tariff with ONLY semi-punta bands would be classified as flat and get no TOU horario. Not currently possible (semi-punta is supplemental), but the inconsistency between ENERGY_FIELDS and the isTou fields is a latent bug.

divisionsPersisted undercounts when a zone is skipped for one tariff but succeeds for others

services/utility/tariffs/cfe/src/handlers/tariff-persist.lambda.ts:176

divisionsPersisted = [...persistedZones].filter((z) => !skippedZones.has(z)).length subtracts any zone skipped under ANY rate type. no_energy_component and component_regression are content-specific: a zone could have prices for GDMTO but a malformed row for GDMTH. In that case the zone IS persisted for GDMTO, but skippedZones from GDMTH causes it to be excluded from divisionsPersisted — tipping a completed result to partial_success incorrectly. Conservative (safe direction) but misleading for operators.

runPersist function body wrapped in a redundant block statement

services/utility/tariffs/cfe/src/handlers/tariff-persist.lambda.ts:82

Refactoring artifact from extracting the withDb callback. Syntactically harmless but confusing. See the low/conventions finding above.

alreadyFailed shell path returns a synthetic event with id: ''

domains/utility/src/tariff-job/tariff-job.shells.ts:244

See the low/conventions finding above. Harmless today; fragile for future callers.

Month availability check uses non-padded string comparison

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:482

availableMonths.includes(String(month)) where month = Number(monthStr) converts '08' to 8 then to '8'. If CFE ever zero-pads month values ('01'..'12'), this check would falsely throw 'CFE has not published 2026-08'. Low risk given the documented CFE behavior, but normalizing to integer comparison would be robust.

TLS certificate verification disabled globally on the Lambda dispatcher

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:744

setGlobalDispatcher(new Agent({ connect: { rejectUnauthorized: false } })) disables TLS verification for all subsequent undici requests in the Lambda process. Carries over from the bill-collector pattern (CFE/Imperva cert chain trips strict TLS). Not an exploitable MITM vector given the hardcoded CFE destination. However the blanket bypass means any transitive dependency making an undici request also skips verification. Consider scoping the relaxed agent to only the CfeClient instance rather than replacing the global dispatcher.

GITHUB_OUTPUT injection via SQL-derived detail field in watchdog workflow

.github/workflows/tariff-coverage-watchdog.yml:77

The detail variable is populated from SQL query output (tariff code and zone code strings), written to $GITHUB_OUTPUT as a free-form value, and later embedded without sanitization in a printf that builds the GitHub issue body. If a tariff code or zone code contained a newline or = character it could inject additional key-value pairs into $GITHUB_OUTPUT (GHSA-cf85-4q5k-3vq). In practice negligible (tariff codes are known short strings), but the fix is trivial: use a heredoc delimiter per GitHub's recommended pattern.

No IAM policy grants on the new Lambda stack — correct posture for Postgres-only Lambdas

infra/cdk/src/stacks/services/utility/tariffs/cfe/lambda.stack.ts:102

The three new Lambdas receive only POSTGRES_URL (from SSM via CloudFormation dynamic reference). No explicit AWS-service grants are added. SFN wires grantInvoke implicitly through the EventBridge rule target. This is the correct minimal-IAM posture. No finding.

API endpoint correctly gates on isPlatformAdmin

apps/platform/src/api/handlers/tariff-jobs.handler.ts:50

POST /tariff-jobs checks authReq.auth.isPlatformAdmin after withAuth. Missing profileId returns 401; non-admin returns 403. Correct — only a platform admin can trigger a global tariff scrape.

rate-pages.json URLs all hardcoded to app.cfe.mx — no SSRF risk

services/utility/tariffs/cfe/src/config/rate-pages.json:1

All nine entries resolve to https://app.cfe.mx/Aplicaciones/CCFE/Tarifas/... An unknown rateType records UNSUPPORTED_RATE_TYPE and scrapes nothing. No SSRF surface.

DB connection string passed via SSM dynamic reference, not hardcoded

infra/cdk/src/stacks/services/utility/tariffs/cfe/lambda.stack.ts:92

POSTGRES_URL is set to a CloudFormation dynamic reference pointing at an SSM String parameter. Follows documented repo convention. No secrets committed to config.

Watchdog correctly masks Postgres connection string before any logging

.github/workflows/tariff-coverage-watchdog.yml:65

The workflow calls echo ::add-mask::$PGURL_RAW immediately after receiving the secret, also masks the :6543-to-:5432 rewritten form, and strips connection string URLs from psql error output via sed before writing to $GITHUB_OUTPUT. Well-handled.

No credentials or secrets in test fixtures

services/utility/tariffs/cfe/__tests__/

All four new test files use only in-memory stub HTML and vitest mocks. No real CFE credentials, Postgres URLs, AWS keys, or API tokens appear in any fixture.

Orphaned JSDoc on hasEnergyComponent — documentation attached to wrong function

domains/utility/src/tariff-rate/tariff-rate.scrape.ts:109

Two consecutive JSDoc blocks appear before droppedComponents. The first block (lines 109-126, 'Does this scrape carry a price for ENERGY…') was written for hasEnergyComponent but was placed directly before droppedComponents, so it documents the wrong function. TypeScript/IDEs attach a JSDoc to the immediately-following declaration; the second block correctly documents droppedComponents. hasEnergyComponent ends up with no JSDoc at all. The first JSDoc must move to immediately precede hasEnergyComponent.

Spurious extra block scope inside runPersist body

services/utility/tariffs/cfe/src/handlers/tariff-persist.lambda.ts:82

runPersist opens with an unnecessary bare block { at line 82 (matched by } at line 186), a refactoring artifact from extracting the withDb callback. Syntactically valid but misleading — readers may wonder if the outer function has a reachable implicit undefined return. Remove the superfluous brace pair.

Fake outbox event (id: '') returned in alreadyFailed branch of failTariffJobShell

domains/utility/src/tariff-job/tariff-job.shells.ts:247

When decision.value.alreadyFailed is true the shell returns ok({ job, event: { id: '', eventType: ... } }). The synthetic event satisfies ShellResult but carries an empty id that would mislead any future caller that stores or logs the event id. Current callers only check .ok and discard the value. A null event or dedicated alreadyFailed: true variant in the return type would make the intent explicit.

as never in test files — established codebase pattern for closed interfaces

domains/utility/src/tariff-rate/__tests__/tariff-rate.scrape.test.ts:98

Tests pass {} as never to circumvent TypeScript excess-property checks on the closed RateComponents interface. Same pattern exists in other integration tests. No change required.

resolveTargetYearMonth throws instead of returning Result — acceptable in Lambda shell layer

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts:59

Throws in Lambda handlers are the accepted mechanism for the imperative shell per FCIS. Domain decisions and shells on the other side of the boundary are correctly pure.

rateTypes now required in CreateTariffJobApiInputSchema — intentional breaking change, well-documented

packages/api/src/schemas/tariff-job.schemas.ts:23

Removing .default(['GDMTH']) makes rateTypes required (422 if absent). The JSDoc explicitly calls this out and cites BAT-308. Correct and intentional.

VALID_TRANSITIONS correctly extended to allow queued → failed

domains/utility/src/tariff-job/tariff-job.decisions.ts:25

Adding 'failed' to the queued transition set is correct: a scheduler reaper needs to mark jobs that die before they start. The FSM comment explains the WHY. decideFailJob is now idempotent on an already-failed job.

findActiveByYearMonth uses sql template — consistent with existing tariff-job.queries.ts pattern

domains/utility/src/tariff-job/tariff-job.queries.ts:83

Uses sql`${tariffJobs.config}->>'yearMonth' = ${yearMonth}` for JSONB field extraction, matching the pattern already established in hasActiveJobForYearMonth. Drizzle parameterizes yearMonth correctly. No injection risk.

droppedComponents (SG-4) path not exercised at the persist-handler level

services/utility/tariffs/cfe/__tests__/persist.test.ts:64

The component_regression skip reason fires when droppedComponents(existing, incoming) is non-empty. tariff-rate.scrape.test.ts unit-tests the pure function, and tariff-rate.shells.ts contains the guard — but persist.test.ts never supplies a `findExistingRate` mock that returns a populated row AND an incoming scrape that drops a field. There is no end-to-end test that exercises findExistingRate → droppedComponents → skipped[component_regression] → persist handler counts. The guard is the newest SG-4 protection; without a handler-level test, a refactor of runPersist could silently disconnect it.

no_energy_component skip path not tested end-to-end at the persist-handler level

services/utility/tariffs/cfe/__tests__/persist.test.ts:65

persist.test.ts exercises unmapped_division via mock skip returns, but never exercises no_energy_component. The persistScrapedRatesShell contains a distinct message branch for this reason; the handler's skip-counting and failure-classification logic paths for no_energy_component are untested. A regression in how this skip surfaces in the job result would be invisible.

classifyHorarioMatrix and extractHorarioRows not directly tested — only via runCollection stub

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:318

extractHorarioRows and classifyHorarioMatrix are private functions exercised only indirectly through runCollection with a well-formed stub page. Their boundary conditions (empty table, missing bands, malformed rows) are not independently tested. A targeted test or export of these functions would make the horario parsing logic more robust to CFE page changes.

VDM table-to-zone pairing order not pinned against real positional swaps

services/utility/tariffs/cfe/__tests__/collector.test.ts:374

The config-integrity test asserts containedDivisions.map(divisionToZoneCode) equals ['DL', 'DM', 'DN'] (exact order), but the collector test for VALLE DE MEXICO does not assert specific zone codes on the output. If CFE ever reorders the table rows for VDM, the config-integrity test passes but the zone codes in the output would be swapped — DL prices written under DM, etc. The collector test should assert zone code values, not just entry count.

Watchdog workflow references STG_POSTGRES_URL — secret may not exist in the repo

.github/workflows/tariff-coverage-watchdog.yml:44

The new tariff-coverage-watchdog.yml uses url_secret: STG_POSTGRES_URL for the staging matrix entry. The canonical secrets manifest in .claude/rules/infrastructure.md lists PROD_POSTGRES_URL for prod but does not document STG_POSTGRES_URL. If this secret is absent from the repository secrets, the staging matrix job will fail silently on every cron run — making the watchdog fire spuriously or not at all for staging.

persistSchedules failure path from runPersist is not covered

services/utility/tariffs/cfe/__tests__/persist.test.ts:160

persist.test.ts tests the case where persistSchedules is skipped (empty schedules array). But it never tests the case where persistSchedules returns an err(...) result. The handler's failure-accumulation and partial_success classification for a schedule-persist failure is untested.

th-label fallback (td[0] for Capacidad/Distribución) exercised by stub but no dedicated assertion

services/utility/tariffs/cfe/__tests__/collector.test.ts:254

The collector's extractRateCellTables has a fallback: when a row has no <th>, it uses td[0] as the label. The stub emits this structure and the integration works, but no test asserts the fallback specifically. A targeted test with a th-less row would pin this behavior.

decideFinalizeJob: divisionsCollected > 0 but divisionsPersisted = 0 with no failures is not tested

domains/utility/src/tariff-job/__tests__/tariff-job.decisions.test.ts:136

The decideFinalizeJob tests cover: all persisted → completed, some failed/some persisted → partial_success, none persisted with one failure → failed. The edge case where divisionsCollected > 0 but divisionsPersisted = 0 and failures is empty (all skipped, none failed) is not covered.

parseRateValue does not test null/undefined input directly

domains/utility/src/tariff-rate/__tests__/tariff-rate.scrape.test.ts:46

parseRateValue has if (raw == null) return null at the top, guarding against null/undefined callers. The test only covers empty string, '-', and 'N/A'. A test with parseRateValue(null as unknown as string) would pin this guard explicitly.

Overall test architecture is well-suited — pure functions unit-tested, orchestration dependency-injected

runCollection and runPersist are exported from their Lambdas and accept injected dependencies (RatePageClient, PersistDeps), making them testable without network or DB. The four new test files cover the critical parsing and orchestration paths. Good architectural choice.

N+1 DB queries for the droppedComponents guard — 17 queries per rate type per persist call

domains/utility/src/tariff-rate/tariff-rate.shells.ts:344

persistScrapedRatesShell calls tariffRateQueries.findByTariffZoneAndPeriod once per rate (zone) inside a sequential loop over input.rates. With 17 zones per tariff and 9 tariffs fanned out across 9 Persist Lambda invocations (one per rateType), each invocation issues 17 sequential DB round trips before any upsert. A single bulk query (WHERE tariff_id = X AND pricing_zone_id IN (...) AND year_month = Y) would collapse this to 1 round trip per persist call and is directly expressible in Drizzle.

Orphaned JSDoc comment without a function in collector test

services/utility/tariffs/cfe/__tests__/collector.test.ts:146

A standalone JSDoc comment at line 146 ('Records every URL fetched so a test can prove one page wasn't reused for another tariff.') is immediately followed by a DIFFERENT function (selectionFrom). The comment documents a helper that was moved or renamed during refactoring. Remove or reattach it.

Redundant delete formState[NAMES.division] before and after state postback in scrapeDivision

services/utility/tariffs/cfe/src/handlers/tariff-collector.lambda.ts:520

Inside scrapeDivision, formState[NAMES.division] is deleted three lines before post(baseResp, NAMES.state, ...), then deleted AGAIN between the state postback and the municipality postback. The second delete is redundant (the first already removed the key). Remove the duplicate.

CreateForCurrentMonth SFN state is a functional duplicate of CreateFromCron with monthsBack: 0

infra/cdk/src/stacks/services/utility/tariffs/cfe/stepfunctions.stack.ts:195

CreateForCurrentMonth passes monthsBack: 0 to the transition Lambda; CreateFromCron passes $.monthsBack — both invoke the same create_from_cron handler path. The two are semantically identical when monthsBack=0. Unless there is a trigger-path distinction, these could be collapsed into a single state with an input override.

DIVISION_FIELD constant used before its declaration in test file — potential TDZ issue

services/utility/tariffs/cfe/__tests__/collector.test.ts:155

selectionFrom on line 155 references DIVISION_FIELD, but DIVISION_FIELD is declared on line 190. In a TypeScript const context this is a temporal dead zone: the initializer of selectionFrom is evaluated at module load, before DIVISION_FIELD is bound. In practice, vitest evaluates the module synchronously and hoists const declarations, so this works — but it is fragile. Reorder the declarations.

aggregatePersistResults divisionsDiscovered sum is correct for fan-out

services/utility/tariffs/cfe/src/handlers/tariff-transition.lambda.ts:87

divisionsDiscovered sums per-rateType pair counts correctly across fan-out branches. Each branch reports its own pair count; the aggregate gives total (tariff × zone) pairs handled. No issue.

STG_POSTGRES_URL reference in watchdog may not exist — see medium/tests finding above

.github/workflows/tariff-coverage-watchdog.yml:46

Duplicate of the tests/medium finding. The watchdog's staging matrix job will silently fail if this secret is absent.