fix/rpu-unique

ensureContractForTariff CreateContractForTariff arm now crashes with 23505

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

The CreateContractForTariff branch inserts a new contract with status='active' for the same contract_number (RPU) as the existing source contract, which remains active. The new partial unique index fires a 23505 on that insert because both rows share the same contract_number with status='active' and deleted_at IS NULL. The shell does NOT terminate the existing contract before inserting. Currently dead code (not called from outside the domain), so no prod impact today — but any future caller reaches an unhandled crash. Fix: terminate the existing contract before inserting, mirroring the TerminateAndReplace branch.

insertContractRaceSafe misidentifies new-index violation as compound-key collision

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

When a 23505 fires on contractQueries.insert, insertContractRaceSafe assumes it came from idx_utility_contracts_compound_key and calls findByCompoundKey to recover. If the violation instead came from uq_utility_contracts_one_active_per_rpu (e.g., two concurrent CreateContractFromBills calls for the same RPU with different tariff codes — different compound keys, same contract_number), findByCompoundKey returns null and the shell receives { contract: null, adopted: true }, triggering ContractErrors.databaseError('insert') — a 500. In practice OCC serialises these paths so it's unlikely to fire here, but when it does the error is opaque and misleading.

demotedContract in return value carries stale version and status

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

In the ReactivateContract branch, `demoted` is assigned from d.demoteContract (the pre-update snapshot). After contractQueries.updateWithVersion succeeds, the DB row has version=demoted.version+1 and status='terminated', but EnsureContractFromBillsResult.demotedContract still carries { status: 'active', version: demoted.version }. No current caller reads demotedContract.version/status to drive a subsequent write, so this is latent. Fix: capture the updateWithVersion return value and expose the updated row.

Concurrent wizard double-submit now surfaces as unhandled 23505 → 500

domains/cross-domain/src/contract-wizard.shells.ts

Pre-PR, wizard contracts with null tariffId/serviceId were exempt from the compound unique index (NULLs not equal in Postgres unique indexes). Post-PR, the new partial index on contract_number fires on concurrent wizard double-submits that both pass the PATH-1/PATH-2 read checks before either tx commits. The 23505 bubbles as a 500; rollback is correct so no data corruption. Adding SAVEPOINT-based recovery similar to insertContractRaceSafe would convert this to a clean conflict response.

Demote-first write order correctly handles non-deferrable constraint

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

PostgreSQL checks non-deferrable constraints per-statement. Terminating the old active row first, then activating the target ensures the constraint holds after each individual statement. Correct implementation.

INSERT RLS withCheck:true enables DoS via intentional index collision

packages/database/src/schema/utility-contracts.ts:124

The INSERT policy for authenticated users has withCheck: sql`true` — no org-scoping on writes. Any authenticated user who knows a valid CFE contract number can INSERT a row with status='active', causing a 23505 that blocks ensureContractFromBillsShell from activating the legitimate contract for that RPU. The pipeline runs as service_role (bypasses RLS), but the unique constraint is enforced regardless. This turns an incomplete rogue INSERT into a persistent blocking condition for bill collection. The permissive INSERT policy pre-existed this PR; the new unique index gives it concrete impact.

UPDATE withCheck is unconditionally true — pre-existing RLS gap amplified

packages/database/src/schema/utility-contracts.ts:125

The UPDATE policy's withCheck: sql`true` imposes no constraint on values written. An authenticated user with read access to a contract (via SUC → site in their org) can UPDATE that row's status to 'active' directly via PostgREST. Combined with the unique index, a successful status escalation can pin the RPU in a blocked state for the pipeline. Pre-existing gap, but the partial unique index now gives it concrete, observable impact.

Partial index predicate uses string literal — consistent with schema enum

packages/database/drizzle/0061_dusty_ben_parker.sql

WHERE clause uses string comparison against a text column (status uses text enum, no DB-level type). Safe and consistent with other partial indexes in the table. No injection risk — this is DDL, not dynamic SQL.

ensureContractForTariff missing 'Shell' suffix (pre-existing violation)

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

Canonical-form naming requires shell functions to be named {operation}Shell. `ensureContractForTariff` follows the full FCIS shell pattern but is missing the suffix. Pre-existing violation not introduced here; the companion ensureContractFromBillsShell is correctly named.

Migration SQL and _journal.json missing trailing newline

packages/database/drizzle/0061_dusty_ben_parker.sql:1

0061_dusty_ben_parker.sql ends without a newline (POSIX text-file requirement). _journal.json also has no trailing newline. Generated files, but inconsistent with other migrations in the repo and can cause issues with psql \i or diff tooling.

No test directly exercises the new DB constraint

domains/utility/src/utility-contract/__tests__/ensure-contract-evidence.integration.test.ts

The partial unique index uq_utility_contracts_one_active_per_rpu is proven to exist by the migration but its enforcement is never tested. No integration test attempts to create a second active row for the same contract_number and asserts a 23505. A future migration error or typo in the index predicate would be invisible. Minimal gap-filler: insert contract A (active), attempt to insert contract B with same contract_number and status='active', expect DB throw with code 23505.

No test for 'demote succeeds, reactivate fails' OCC rollback path

domains/utility/src/utility-contract/__tests__/ensure-contract-evidence.integration.test.ts

The PR swapped write order to demote-first, reactivate-second. Existing OCC test only covers demotion updateWithVersion failing (stale version) → tx rolls back. There is no test for: demote succeeds but reactivate updateWithVersion returns null (concurrent writer modified the terminated row between FETCH and reactivation). In that scenario the whole tx rolls back — including the demote — leaving the RPU with its original single active row. This rollback is the sole safety net against a zero-active-contract state after the write-order change, and it is not covered.

23505 from UPDATE (reactivate step) untrapped — surfaces as 500, not 409

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

insertContractRaceSafe catches 23505 for INSERTs but the reactivation updateWithVersion is an UPDATE with no analogous 23505 guard. If a concurrent tx activates another row for the same RPU between the demote and reactivate writes, the index fires on the UPDATE and the unhandled exception surfaces as a 500. The PR acknowledges the manual updateContractShell path's 23505→500 gap but does not track the equivalent hazard in the ReactivateContract shell. This sub-path lacks both a guard and a test.

OCC rollback test comment describes old write order — misleading after swap

domains/cross-domain/src/__tests__/cfe-job-intent.integration.test.ts

An inline comment says 'the reactivation that ran BEFORE the failed demotion was rolled back'. After the write-order swap (demote-first, reactivate-second), the reactivation has not yet run when the demotion fails — there is nothing to roll back on that side. Test behaviour is correct but the comment will mislead future maintainers about the failure sequence.

`let demoted` alias is unnecessary indirection — simplify to d.demoteContract

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

`demoted` is always equal to `d.demoteContract` (set on line 900, never mutated). The `if (demoted)` guard at the attachment-transfer block is semantically identical to `if (d.demoteContract)`. A reader must trace the let/assign/re-check to confirm they're the same. Simpler: drop the let, use d.demoteContract directly in the first block, and use `if (d.demoteContract)` for the attachment-transfer guard. The alias was needed in the old code because `reactivated.publicId` was referenced (a bug — reactivated not yet in scope); the PR fixed that bug by using d.contract.publicId but left the aliasing pattern in place.

Mixed .where() styles on adjacent uniqueIndex declarations

packages/database/src/schema/utility-contracts.ts:97

The preceding compoundKeyIdx uses Drizzle-native isNull(table.deletedAt) for its partial WHERE. The new oneActivePerRpuIdx uses a raw sql`` template for a compound condition. Both produce identical SQL. The compound condition requires raw sql (Drizzle partial-index API only accepts one expression), so a short inline comment would pre-empt the 'why not isNull()?' question.

suspended status not covered by partial index — invariant is application-enforced for that state

packages/database/src/schema/utility-contracts.ts

The index predicate covers only status='active'. A suspended row doesn't block a second active row from being created. In the current flow the decision layer always routes suspended rows through TerminateAndReplace first, so no live bug. Worth noting that the DB only backstops the active state, not suspended.

SEO lens skipped — no apps/web changes

n/a

Diff contains no marketing website files. SEO lens is a no-op for this PR.