[bokelley]

Closes #1492

What

Mirrors the cross-SDK contract that merged in adcp-client#3606 (closes adcp-client#3281): the per-object advisory lock adcp-reporting-object:{binding}:{object_ref} is removed where nothing pairs on it, and kept in the two places that pair with each other. It is a partial removal, not a full one, because removing it everywhere is unsafe (the collector case below).

Contract (as implemented)

1. No lock: retirement, the intent sweep, and a commit that adopts an object its own intent recorded. Per object, one statement checks that no live or unavailable row set outside the operation (outside the obligation for retirement, none excluded for the sweep) names the same binding, object_ref and native_version_ref, and no open intent in the binding lists it in created_objects (current and pre-alignment objectKey/nativeVersion spellings both read); then the exact version is deleted. No transaction or pooled connection is held across the provider delete.
2. Lock key text unchanged: adcp-reporting-object: + binding ID + : + object ref, hashed in SQL with hashtextextended(text, 0), so a Python deleter and a JS adopter agree in one database.
3. A commit takes the account lock, then closes its intent with DELETE ... WHERE owner_token = ... AND state = 'open' RETURNING created_objects (no row fails STATE_UNAVAILABLE). For each adopted object whose (object_ref, native_version_ref) is not in the returned list it takes its binding's object lock, in object-ref order, then proves every adopted object with a version-pinned stat (full-read fallback), then inserts the chunk rows.
4. The orphan collector (delete_orphaned_object) locks every binding's key for the object (configured plus registered, sorted by binding ID), repeats the reference check across all bindings under those locks, then deletes. It is the only locked delete. A Python deleter holding every binding's lock pairs with a JS adopter holding its own binding's lock.
5. Lock order: the account lock, then object locks. Native versions must be write-unique (write-unique-version in provider conformance).

Precondition, documented and not enforced: two writable bindings must not share one bucket and prefix, because the reference check is per binding outside the collector (the old per-binding lock keys did not order those either).

Why it is safe without the lock

Fences that exist independent of the lock: (A) the account lock held by a commit, retirement phase 1, intent takeover and the 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, so for any two deleters at least one deletes; (C) the intent moves open -> sweeping against the commit's DELETE ... WHERE state = 'open', in the same transaction as the chunk 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} and {content_sha256}, so only one row set can name it. The only delete no fence covers is the collector's, of an object no record names, against a commit adopting it unrecorded; that pair keeps the lock on both sides. The full case table is the one in adcp-client#3606, and the contract is written in ReportingRowStorageV1 (# -- object lifetime) and docs/reporting-row-storage.md#the-object-lock-contract.

Python-specific notes

• Python's compaction does not exist, so only retirement and the sweep lose the lock. Retirement phase 1 already commits pruning under the account lock before phase 2 deletes, which the comment in row_retention.py now records.
• delete_unreferenced no longer takes any_binding; delete_orphaned_object and _is_referenced replace it. lock_and_verify_adopted gains a recorded argument (default empty, which locks every adopted object as before, so an existing caller keeps the old behavior). recorded_intent_objects reads both key spellings.
• No !: no exported name or wire/schema change, and rolling upgrades between a version that locks everywhere and this one are safe because every pair that needs exclusion keeps the lock on both sides. perf because this removes work from commit and retention paths.

Deployment, migration and rollback

No migration and no schema change. Deploy as usual. Rollback is a plain binary rollback in either direction at any time: the lock key text is unchanged, and the older code locks a superset of what this code locks.

Tests

Ported from JS reporting-row-shared-object-pg where they apply (tests/conformance/reporting/test_reporting_row_object_lock_contract_pg.py):

• A delete (retirement-scoped) completes while the object lock is held elsewhere and holds no connection across the provider delete (checked through pg_stat_activity from inside the provider delete).
• A referenced object is kept; a pruning row set does not count.
• The intent sweep completes while the lock is held elsewhere.
• Adoption of an object the intent recorded takes no lock; both key spellings are read; adoption at a different native version, of an object the intent does not name, or with no intent still takes it and then proves the object.
• An orphan delete in flight blocks an adopter of the unrecorded object, which then fails cleanly on the version-pinned stat; an orphan delete takes every binding's lock and keeps an object another binding's intent names.
• A writer whose intent expired and was swept cannot commit the swept objects.
• The lock text is the one the JavaScript SDK hashes.

Not ported because they exercise JS-only paths: compaction (Python has none), the header-only sharer losing to compaction, and the readOnly-legacy retirement barrier test. The existing Postgres suites for row storage, retention, retirement, the collector and the fence pass unchanged.

make lint and make typecheck-all pass; Postgres tests run locally, serially.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Approve

Approve — concurrency-critical lock-removal refactor, verified safe with no blocking or medium findings.

What I checked:

• gated_paths is false; review_decision is REVIEW_REQUIRED but no gated path or prose hard gate applies (this is a separate reporting-storage package, not the public adcp.* surface). Row 2 does not fire.
• high_risk is false; no deletions, no sensitive-category findings.
• No author no-auto-approve team match.
• Reviewer found nothing Critical/High/Medium. The lock removal in delete_unreferenced is load-bearing-fence-enforced (assert_reporting_row_key_template_v1 requires {revision_key}+{content_sha256}), the retained lock on delete_orphaned_object vs lock_and_verify_adopted is correct, lock order is consistent (no cycle), and signature changes are back-compatible. The perf: commit prefix is correct — no wire/schema/public-export change. A 293-line PG conformance suite covers the exact races.

One non-blocking follow-up noted by the reviewer (stale "atomic step" comment at row_retention.py:144-145) is outside the diff and informational only.

No row 1–8 condition fired → row 9 approve.

[aao-secretariat]

Ladon verdict: Approve

Approve.

Clean port of the cross-SDK lock contract (adcp-client#3606). The reviewer verified the safety rests on enforced invariants: _assert_binding_key_template requires {revision_key}+{content_sha256} on writable bindings (fence E), so lock-free deletes cannot race a same-key adopter within a writable binding; cross-binding stays on the orphan collector's any_binding+lock path. The per-object advisory lock is correctly dropped on retirement/sweep/recorded-adoption and kept only on the two paired paths (delete_orphaned_object, lock_and_verify_adopted). Confirmed: DELETE ... RETURNING is single-row (PK match), so fetchone()-is-None equals the old rowcount != 1; the int>0→bool migration in orphan_collector is correct; the lock-free paths no longer hold a connection across provider I/O. A comprehensive conformance suite was added.

No critical/high findings. No gated paths, no high-risk flag, no author-team gate. Only a single Low finding (stale atomicity comment at row_retention.py:144, not even in the diff), which does not block.

Medium findings
None.