Conversation
Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>
BinaryBourbon
left a comment
There was a problem hiding this comment.
Approving.
Conversation FOR SHARE before the insert is the right lock: it is strong enough to stabilize ownership and compatible with the FOR KEY SHARE the allowance's own foreign key needs, so the lock does not fight the insert it is protecting.
The :ownership race is the case I would not have thought to write and it is the valuable one — it proves the conversation is re-read under the lock rather than trusted from before the wait, so a row that changed owner while the writer was blocked produces {:error, :not_found} and no audit row. The :duplicate half proves the PK constraint still wins after a lock wait rather than the transaction retrying blindly.
Audit is outside the transaction, metadata is control names only, and Enum.filter(ExecutionLimits.keys(), ...) gives a stable order so the assertion is not seed-dependent.
Returning the same {:error, :not_found} for missing and foreign conversations before any request validation is the correct disclosure boundary — a caller cannot use a malformed request to learn whether an id exists.
Adding both this and narrow_execution_allowance/3 to audit_guardrail_test.exs rather than the exclusion list is the right choice.
Combines the reviewed execution-limit admission stack (#1773-#1786) into one change: a typed policy resolver, versioned per-conversation allowance storage, owner-scoped create/narrow operations, and a refusal at every door that could otherwise accept a limit this deployment cannot enforce. Nothing is enforced yet, by design: every control is refused via require_controls(limits, []), so a saved allowance is always empty today. The stack makes it impossible to accept a limit the runtime cannot honour, ahead of the work that will honour it. Excludes #1787 (host ceilings) and #1789-#1791, which have changes requested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzUPkXKdmnZYeJ1sqjpnp9
Initial execution allowances only had an unscoped storage changeset. Add
Conversations.create_execution_allowance/3to lock the conversation's ownership through insertion and record a successful creation in the audit trail. Missing and foreign conversations return the same error before validation. Duplicate inserts cannot replace a saved policy, revision or timestamp.Stacked on #1783. Four files, +245/-1; 44 added context lines, with tests and storage documentation making up the rest.
Validation: 125 related tests passed, including observed PostgreSQL lock waits for a competing insert and an ownership change. Tests also cover redacted audit metadata, invalid inputs, unchanged active work and refusal of unsupported execution. Full precommit passed 4,786 tests and 6 doctests with zero failures. CI passed.
This is internal persistence, with no launch or HTTP caller. Admission must still resolve trusted current ceilings and verify runtime support before using it. Account-ceiling preflight remains on sibling #1785; host settings and complete runtime enforcement remain separate.
Extraction: #1745, #1754. Live acceptance remains #1732.