[bokelley]

Closes #3281

What changed

The per-object advisory lock adcp-reporting-object:{binding}:{objectRef} is removed from retirement, compaction and the write-intent sweep, and from commits that adopt an object their own intent recorded. It stays in two places that pair with each other, because nothing else orders them: the orphan collector's delete, and a commit that adopts an object its own intent did not record. Decision: partial removal, not full removal and not a close-with-analysis (removing it everywhere is unsafe; see case 3 below).

What it buys: retirement, compaction and the sweep no longer run each object as a transaction holding a pooled connection and an advisory lock across the provider delete (the check is a plain query, then the version-pinned delete). A retry that adopts its own recorded object pays no lock round trip.

Proof, case by case

Fences that exist independent of the lock: (A) account lock held by the commit, retirement phase 1, compaction phase 1, intent takeover and sweep claim; (B) rows_state moves to pruning under the account lock before any delete, and pruning row sets neither count as references nor can be shared; (C) the intent moves open -> sweeping against the commit's DELETE ... WHERE state = 'open', which is in the same transaction as the chunk-row INSERT (so a reader sees the open intent or the chunk rows, never neither); (D) deletes name an exact native version; (E) every writable key includes {revision_key} (sha256 of the row set ID) and {content_sha256}, so one row set alone can name a key.

Case	Protected by	Lock needed?
Header-only revisions sharing chunks (rows_shared_from_row_set_id)	A + B. The sharing commit takes the account lock and FOR SHAREs a predecessor that must be live; retirement/compaction mark it pruning first. The sharer commits first (its chunk rows make the check skip) or fails STATE_UNAVAILABLE and re-uploads under its own key. The lock never covered this: the sharing path took none.	No
Writer retrying its own row set, adopting its own recorded object	The open intent names the object until the commit; the sweep cannot claim an intent a takeover renewed (C); key is unique to the row set (E).	No (only when the intent records the object at that exact ref and version)
Intent sweep vs a writer alive past its lease	C. Exactly one of the sweep claim and the commit's intent close wins. A swept writer's commit fails; a retry cannot take over a sweeping intent.	No
Retirement vs retirement, compaction vs retirement, readOnly bindings with old templates sharing objects	B. Each deleter commits pruning before its check and counts only non-pruning row sets, so for any two at least one deletes (commit-order argument in the TSDoc on deleteUnreferencedObjects). readOnly bindings never take commits, so nothing adopts through them.	No
Legacy rows written before {revision_key} was required	They live only in readOnly bindings (writable ones are refused at config time), so no new write can collide with them (E needs a 256-bit coincidence).	No
Orphan collector vs a commit adopting an object no record names (an earlier attempt's PUT landed before it was recorded, or was created after a takeover)	Nothing but the lock. The collector's reference check and delete are not atomic against the adopter's stat + chunk insert.	Yes, kept on both sides
Collector and its clone fence (#3593)	The fence protects across databases; an advisory lock is per database and could never order a clone against the original. Unchanged.	n/a

Known precondition, pre-existing and not changed by this PR: bindings that share one bucket and prefix must route a row set to the same binding across attempts. The old lock keys were per binding, so they did not order A's sweep against B's adopter either. Now documented in REPORTING-LEDGER.md.

Final contract (for the Python SDK, adcp-client-python#1492, to mirror)

1. Retirement, compaction and the intent sweep take no object lock and hold no transaction or pooled connection across the provider delete. Each object: one statement checking that no live/unavailable row set outside the operation (outside the obligation for retirement, outside the row set for compaction, none excluded for the sweep) references the same binding, object_ref and native_version_ref, and no open intent in the binding lists it in created_objects (read both object_ref/native_version_ref and the pre-alignment objectKey/nativeVersion); then delete that exact version.
2. Lock key text, unchanged: adcp-reporting-object: + binding ID + : + object ref, passed to pg_advisory_xact_lock(hashtextextended(text, 0)).
3. A commit that adopts existing objects: take the account lock; close the intent with DELETE ... WHERE ... AND owner_token = $ AND state = 'open' RETURNING created_objects (zero rows means fail STATE_UNAVAILABLE); for each adopted object (sorted by object ref) whose (object_ref, native_version_ref) is not in the returned created_objects, take the lock for its own binding; then prove every adopted object with a version-pinned stat (full read fallback if the provider has no stat); then insert row set and chunk rows. Objects the intent recorded take no lock.
4. The orphan collector's delete is the only locked delete: in one transaction, take the lock of every binding (configured plus registered), sorted by binding ID; repeat the reference check with every binding included; delete the exact version. Lock order everywhere: account lock, then object locks.
5. Native versions must be write-unique (provider conformance write-unique-version).

Tests

reporting-row-shared-object-pg.test.js: the old "a delete waits for the object lock" test is inverted. New: retirement, compaction and the sweep complete while the object lock is held elsewhere and hold no pooled connection across the delete; self-retry adoption (current keys and pre-alignment keys) takes no lock; adoption with a different native version, or of an object the intent does not name, still takes it; sweep vs an expired-intent writer; takeover renews against the sweep; a retry cannot take over a sweeping intent; an orphan delete in flight blocks an unrecorded adopter, which fails cleanly; header-only sharer loses a race to compaction of its predecessor and re-uploads; two retirements of readOnly legacy row sets held at a barrier until both are pruning always free the object. Mutation-checked: counting pruning row sets in the check, and dropping rows_state = 'live' from the sharer's FOR SHARE, each fail a test. Full reporting-storage suite (parallel, serial, chunked) green against PG; typecheck, lint, prettier, doc-links, package-size green.

Expert review

Run read-only on the diff: security-reviewer, code-reviewer-deep, nodejs-testing-expert, plus npm run review:codex -- --all --base main (its sandbox cannot read the repo, so the diff was piped on stdin; verdicts: protocol "ships", code "no findings", security "safe under the stable-binding contract").

Fixed: proof wording scoped to writable bindings and the replay-race nuance (a late writer can adopt a just-committed object but its commit inserts nothing); stale-deleter safety documented as depending on write-unique native versions; retirePeriod comment says no lock orders the shared-legacy case; RecordedIntentObject TSDoc misplacement; one recordedIntentObjects helper for the two key spellings; a named RowObjectQueryable type; test hook reentrancy and leaks (afterEach reset, hook cleared before it is awaited); pending work awaited after a timeout; parallel readOnly retirement test now forces the worst interleaving with a barrier; test names and no longer live assertion tightened; added the legacy-key and wrong-version adoption cases; "Cross-SDK contract" softened to the intended contract.

Declined: the security reviewer and codex both flagged that the reference check is per binding, so two bindings sharing one bucket, prefix and template could alias a key if a row set is routed to different bindings across attempts. Pre-existing (the old per-binding lock keys did not order those two bindings either), widening the check to every binding would not close a check-then-delete race, and only a lock on every path would. Documented as a precondition instead; enforcing it (refusing two writable bindings that alias one key space) is a configuration-breaking change that belongs in its own PR.

Changeset: patch. No exported API, schema or wire behavior changes, and the lock key text is unchanged, so rolling upgrades between a version that locks everywhere and this one are safe (every pair that needs exclusion keeps the lock on both sides). No migration needed.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Approve

Approve — removes the per-object advisory lock from reporting-storage delete/adopt paths while preserving it for the orphan collector and for commits adopting an object their own intent did not record.

What I checked:

• Lock-free delete at postgres.ts:357 verified safe: phase-1 pruning committed under the account lock orders concurrent retirements/compactions (fence 4), and unique {revision_key} keys ensure only the owning row set adopts via a writable binding (fence 1).
• deleteOrphanedObject (L398) correctly retains the every-binding lock since no record names an orphan.
• Recorded-adopt skip (L493) still proves via assertObjectsPresent (unchanged base).
• Extracted isRowObjectReferenced SQL is byte-identical to the prior inline query — faithful refactor, no wire/shape change.
• Changeset present and correctly scoped (@adcp/reporting-storage: patch); patch level justified — no API/schema/wire change, lock key text unchanged, rolling upgrade keeps the lock on both sides of every pair still needing mutual exclusion.
• ~12 new integration tests cover each side of the contract.

No Critical/High/Medium findings. high_risk is false, gated_paths is false, no author team gate. Falls through to row 9 — approve.