fix/dup-repair

Dry-run display shows 'terminate' for REACTIVATE_NO_EVIDENCE but execution soft-deletes + purges all bills

scripts/repair-dup-contracts.ts:460

The plan display at line ~460 uses `p.verdict === 'FLIP_DELETE_TWIN' ? 'soft-delete' : 'terminate'`. REACTIVATE_NO_EVIDENCE takes the `else` branch in execution (soft-delete + full bill purge, same as FLIP_DELETE_TWIN), not the `FLIP_KEEP_HISTORY` if-branch. So an operator reviewing the dry-run for a REACTIVATE group sees 'terminate' (implying history preserved, no bill purge) but execution will actually soft-delete the row and purge all bills. Fix: change the ternary to `p.verdict === 'FLIP_KEEP_HISTORY' ? 'terminate' : 'soft-delete'`.

REACTIVATE_NO_EVIDENCE skips the in-transaction evidence guard entirely

scripts/repair-dup-contracts.ts:258

The `plan.verdict !== 'REACTIVATE_NO_EVIDENCE'` guard at line 258 skips all in-transaction evidence checking for that verdict. This is logically necessary (no XML exists by census definition). However if a CFE pipeline batch lands an XML bill on the deposed twin between census and the FOR UPDATE lock, the twin now has evidence and the REACTIVATE path would purge a genuine bill. A minimal guard: re-run the evid query and throw if deposedNewest is no longer null on a REACTIVATE plan.

--limit accepts non-integer floats; PostgreSQL rejects LIMIT 5.5

scripts/repair-dup-contracts.ts:114

`!Number.isFinite(LIMIT) || LIMIT <= 0` accepts `5.5`. PostgreSQL rejects `LIMIT 5.5` with 'argument of LIMIT must be integer'. Add `!Number.isInteger(LIMIT)` to the guard.

entity_relationships dedup EXISTS omits source_type from equivalence predicate

scripts/repair-dup-contracts.ts:359

The dedup DELETE EXISTS checks (org_id, source_id, target_type, target_id) but not source_type. Two relationships are logically equivalent only if all five columns match. In practice all utility_contract links have source_type='organization' (confirmed in the shell), so UUID uniqueness makes the gap harmless. A safer condition adds `AND e2.source_type = er.source_type` to the EXISTS subquery.

isTwinShaped extraction is logically equivalent — no regressions

scripts/lib/repair-dup-contracts-core.ts

The extracted `isTwinShaped(c, window)` is byte-for-byte equivalent to both inline checks in the original script for both the REACTIVATE_NO_EVIDENCE and FLIP_DELETE_TWIN paths. The added `window` parameter adds testability without changing defaults.

S3 manifest written before deleteForeignBills — partial-failure risk

scripts/repair-dup-contracts.ts

The S3 manifest file is written (writeFileSync) before deleteForeignBills() is called. If deleteForeignBills throws partway through (per-bill transactions, not a single batch), the manifest lists S3 paths for bills that may not have been deleted from the DB. An operator running `aws s3 rm` from the manifest after a partial failure will delete S3 objects for bills still in the DB, creating dangling rows with missing files. Reversing the order (delete DB rows first, then write manifest) would be safer.

SQL parameterization correct throughout; JSONB path literal is static

scripts/repair-dup-contracts.ts

All user-supplied and DB-sourced values (contract IDs, RPU, public_id) are bound as positional parameters via Drizzle's sql`` tagged template. The jsonpath string is a static literal; plan.rpu is compared against the extracted value as a bind param, not injected into the path expression. No SQL or JSONB injection vector.

All destructive operations correctly inside db.transaction(); snapshot uses 0o600/wx

scripts/repair-dup-contracts.ts

FOR UPDATE lock, evidence re-verify, all UPDATE/DELETE statements across bills, bill_files, utility_contracts, SUC, subscription, and entity_relationships tables are inside the single db.transaction() callback. The snapshot file uses 0o600 permissions and exclusive-create (wx) flag to prevent overwrites and restrict read access.

process.exit(1) in --limit validation skips .finally() cleanup

scripts/repair-dup-contracts.ts:116

`process.exit(1)` at module initialization (line 116, --limit validation) runs before `main()` is invoked and before `.finally(() => connection.end())` is registered. All other early exits in the script use `process.exitCode = 1; return` which lets the cleanup run. If the `@batu/database/client` lazy proxy opens a socket on import, this would leave it open. Prefer: `console.error(...); process.exitCode = 1; process.exit()` or restructure the check after connection is wrapped.

Comment added in a29f8f4c is a useful WHY anchor, not a WHAT restatement

scripts/repair-dup-contracts.ts:277

The comment names both twin-shaped verdicts on the else branch and adds 'twin-shaped by construction' as the semantic guarantee that makes the full purge safe. The commit message confirms motivation: prior review rounds misread REACTIVATE_NO_EVIDENCE as taking the terminate path. This passes the comment quality bar.

Multiple-active group is unhandled and untested

scripts/__tests__/repair-dup-contracts-core.test.ts

`classify` uses `group.contracts.find(c => c.status === 'active')` which silently picks the first active. If two active rows exist, the second is ignored by both the `active` binding and the `terminated` filter — but it IS included in `withEvidence`, so it could become the `holder` while only the first active becomes `deposed`. The result: the second active row is never demoted. The census query almost certainly returns exactly one active per RPU, but a test asserting SKIP_MANUAL (or the current first-active behavior) for a two-active group would document the invariant.

Mutable module-level seq counter in test factory is a shared-state anti-pattern

scripts/__tests__/repair-dup-contracts-core.test.ts:16

`let seq = 0` at module scope is mutated across all tests. No test asserts specific ID values so there is no current flakiness, but adding a test that asserts `contract_id === 'id-3'` would produce nondeterministic results depending on test execution order. Replace with a per-call counter closure or a deterministic factory.

NO_ACTION_OK tie test description implies null-null is a tie, but null-null exits via no-evidence branch

scripts/__tests__/repair-dup-contracts-core.test.ts:64

The test comment 'never flip on equal recency' implies the null=null scenario is also a tie. In reality, when both have newest_xml_pe=null, withEvidence.length===0 and the code enters the REACTIVATE/SKIP_MANUAL branch — it never reaches the tie guard at line 104. The test is correct for non-null equal recency; the null=null case is a different (also correct) code path. Consider a descriptive note to avoid confusion.

Evidence re-verify throws without logging before/after state

scripts/repair-dup-contracts.ts:258

When the in-transaction evidence re-verify detects drift (deposedNewest >= holderNewest), it throws but logs nothing beforehand. The operator's console shows only the error message string with no record of what the census expected vs what was found under lock. Add a console.warn before the guard: `{ rpu: plan.rpu, census: { holder: holder.newest_xml_pe, deposed: deposed.newest_xml_pe }, live: { holderNewest, deposedNewest } }`. Makes the audit trail complete without changing behavior.

Twin bill purge logs no pre-deletion count

scripts/repair-dup-contracts.ts:293

For FLIP_DELETE_TWIN and REACTIVATE_NO_EVIDENCE, the script deletes ALL bills on the deposed contract without logging how many rows it will delete. The dry-run plan shows 'soft-delete' but no bill count. A `SELECT count(*) FROM bills WHERE utility_contract_id = $id` inside the transaction before the DELETE, logged as `console.log('purging N bills from twin ...')`, gives the operator a verifiable audit trail. Particularly useful if a bill arrives on the twin between census and execute.

classify() reduce picks first tied terminated holder non-deterministically

scripts/lib/repair-dup-contracts-core.ts:97

If two terminated contracts share the max newest_xml_pe, the reduce returns whichever appears first in the census array (implicitly ordered by created_at ASC). This is deterministic given stable census ordering, but the implicit tiebreak is undocumented. No prod impact expected given the observed data distribution (57 twin-deletes + 17 flips).