fix/cfe-inputs

updateWithVersion returning null mapped to databaseError(500) instead of versionConflict(409)

domains/utility/src/utility-contract/utility-contract.shells.ts:253

When a concurrent writer wins the optimistic-lock race, updateWithVersion returns null. The shell maps this to ContractErrors.databaseError('update') (a 500-class error), which surfaces as InternalError to the public API caller. The correct semantic is ContractErrors.versionConflict(...) (409 Conflict). This is a pre-existing pattern from updateContractShell (line 181-184) but the new shell inherits it. At minimum, the public handler could retry the shell once on a version-conflict error before returning 500. Low probability in practice (correction runs near the top of the handler before other writes).

Trust horizon comment should clarify bimestral (GDMTH) produces 120-day window

domains/utility/src/cfe-job/cfe-job.decisions.ts:956

The formula is correct but the comment explains it only in terms of 30-day cycles. For GDMTH (bimestral, 60d cycle), the trust window is 120 days — still correct since CFE accepts último (60d) + penúltimo (120d) — but a reader may think '2 cycles' means ~60 days and question why a 4-month-old bill is trusted. Add a parenthetical: '(30d for monthly, 120d for bimestral GDMTH — both are correct: CFE accepts up to penúltimo within their respective cadences)'.

service_name has no length bound in public API parse path (internal surface enforces 200 chars)

apps/platform/src/api/utils/public-v1-validation.ts:420

parseOptionalString (used for service_name) performs no length check. The internal update-contract endpoint validates serviceName via ContractServiceNameSchema (z.string().min(1).max(200).trim()). The public API bypasses that schema, so a caller can send an arbitrarily long service_name that is persisted verbatim. Postgres column type is the only backstop. Add a max-length guard (200 chars, matching ContractServiceNameSchema) in parseJobCreateBody.

latest_total: 0 accepted — conflates caller-supplied $0 bill with internal 0-probe strategy

apps/platform/src/api/utils/public-v1-validation.ts:407

The validation accepts n >= 0. A caller passing 0 bypasses the intent of 'the total of the most recent CFE bill'. The value '0' is also the probe string in the collector's rescue strategy (attemptRegistrationWithRescue), so a caller who deliberately sends latest_total: 0 forces the 0-probe path without triggering the natural rescue. This is low severity because 0-probing already happens internally, but a min value of 1 (or 0.01) would make the API contract unambiguous.

Cross-org scoping improvement confirmed — security regression in old code is fixed

apps/platform/src/api/handlers/public-v1/jobs.handler.ts:149

The old code used findByContractNumber (not org-scoped) for the serviceName fallback. The new code uses findByRpuAndOrgId (org-scoped) in both the update and fallback paths. orgId flows from a validated JWT (withPublicApiAuth). This closes a potential cross-org data exposure. Confirmed improvement.

PERIODS_OF_TOTAL_TRUST should be a module-level constant

domains/utility/src/cfe-job/cfe-job.decisions.ts:961

All named business constants in this file are at module scope: VALID_TRANSITIONS_MAP (line 39), DEFAULT_JOB_CONFIG (line 61), INITIAL_PROGRESS (line 69). PERIODS_OF_TOTAL_TRUST encodes the CFE registration acceptance window ('último OR penúltimo' → 2 cycles), a domain rule worth naming and finding by grep. Move to module scope as a named const. Extract trustHorizonMs computation into a helper or const block to avoid the let/mutation pattern (see LOW finding).

let + mutation for latestTotalTrustworthy diverges from intra-file const-expression style

domains/utility/src/cfe-job/cfe-job.decisions.ts:962

The allCached gate directly below (line 984) uses a const IIFE for an equivalent conditional. The let/mutation form is also what forces the latest!.total non-null assertion (see next finding) — TypeScript cannot narrow `latest` across a mutable variable. A const expression avoids both issues: const cycleLengthMs = latest ? latest.periodEnd.getTime() - latest.periodStart.getTime() : 0; const latestTotalTrustworthy = latest != null && (latest.periodEnd.getTime() + PERIODS_OF_TOTAL_TRUST * cycleLengthMs) > today.getTime();

latest!.total non-null assertion is the only ! in the file — caused by let/mutation indirection

domains/utility/src/cfe-job/cfe-job.decisions.ts:969

The assertion is logically safe (latestTotalTrustworthy is only true inside if (latest != null)), but TypeScript cannot narrow across the let binding. The non-null assertion is a workaround for the let/mutation style. Fixing the const-expression pattern above eliminates this assertion.

updateServiceNameByRpuShell missing named result-type interface

domains/utility/src/utility-contract/utility-contract.shells.ts:224

Every other write shell has a named result interface in the 'Shell Result Types' section (lines 46-71): CreateContractResult, UpdateContractResult, DeleteContractResult, RestoreContractResult. The new shell returns an inline type { contract: UtilityContract | null; updated: boolean }. A named UpdateServiceNameByRpuResult interface belongs in the section and makes the return shape discoverable.

classifyRegistrationFailure test 2 passes wrong input code — accidentally exercises same branch as test 1

services/utility/bills/cfe/__tests__/unit/handlers/collector-lambda.test.ts:89

Test 2 ('TOTAL_MISMATCH WITH a trusted total') passes code: 'REGISTRATION_TOTAL_MISMATCH'. The function strips the REGISTRATION_ prefix (collector.lambda.ts line 943), producing 'TOTAL_MISMATCH', which lands on the same switch-case as test 1 (code: 'TOTAL_MISMATCH'). Both tests exercise the same post-strip branch — a regression that breaks prefix-stripping would cause both to fail (not silently pass), so coverage is not lost, but the test design intent is misleading: the two tests look like distinct scenarios but structurally collapse to one. A third test that passes 'REGISTRATION_TOTAL_MISMATCH' to verify the strip is the correct way to cover that path explicitly.

No unit or integration tests for updateServiceNameByRpuShell — 5 branches uncovered

domains/utility/src/utility-contract/utility-contract.shells.ts:224

The new shell has five distinct return paths: (1) contract not found → ok({updated:false}), (2) serviceName already matches → idempotent ok, (3) decideUpdateContract fails → propagate error, (4) hasChanges false after decision, (5) updateWithVersion returns null → databaseError. None appear in the diff's test additions or in the existing utility-contract integration test file. Per project testing conventions, shells require integration tests for transaction atomicity, outbox event correctness, and error flows.

Missing boundary test for latest_total exactly at the cap (100_000_000)

apps/platform/src/api/utils/__tests__/public-v1-validation.test.ts:305

The rejection test covers 100_000_001 but not 100_000_000. The validation uses strict n > MAX_LATEST_TOTAL so exactly 100_000_000 must be accepted, but this is untested. A future tightening to >= would go undetected. Add parseJobCreateBody({ rpu, latest_total: 100_000_000 }) asserting ok.

latest_total: 0 downstream semantics not tested

apps/platform/src/api/utils/__tests__/public-v1-validation.test.ts:295

A test verifies 0 parses, but no test covers how callerLatestTotal = 0 flows through decideCacheStatus. In the function, callerLatestTotal ?? (...) short-circuits to 0 when callerLatestTotal is 0 (0 is falsy but ?? only gates on null/undefined). Verify: decideCacheStatus with callerLatestTotal=0 should return latestTotal=0, not the stored bill's total.

update+outbox block duplicated between updateServiceNameByRpuShell and updateContractShell

domains/utility/src/utility-contract/utility-contract.shells.ts:248

Lines 248-272 (decide → hasChanges → updateWithVersion → outboxQueries.insert) are a near-verbatim copy of updateContractShell lines 158-208. The two shells emit different event types (updated vs status_changed), which already diverge. An internal helper _applyContractUpdate(tx, decision, actor) could own the write+outbox block and allow both shells to stay synchronized. Small today (~30 lines) but the divergence will grow.

Handler comment implies DB round-trip saving but updateServiceNameByRpuShell runs the same query internally

apps/platform/src/api/handlers/public-v1/jobs.handler.ts:132

Comment says 'Skipped entirely when the caller supplied a name', implying a DB query is avoided. In reality updateServiceNameByRpuShell runs findByRpuAndOrgId inside its own transaction, so the query runs once either way — it's just shifted into the shell's transaction boundary (architecturally correct). Rephrase: 'Skipped to avoid surfacing a stale pre-shell value — updateServiceNameByRpuShell owns the authoritative fetch.'