Should practitioner_services_cap be an AFTER constraint trigger?

[davidtaing]

TL;DR

• The problem — practitioner_services_cap is a before trigger, so it runs ahead of the policy's WITH CHECK: a stranger writing to an approved profile already at three gets 23514 and a 400, where 42501 and a 403 is the honest answer. Nothing leaks and nothing bypasses — the count it reflects is already public.
• The fix — possibly a constraint trigger, which fires AFTER and lets RLS answer first with no authorization logic of its own. Probed green locally at 169/169, and it retires id is distinct from new.id. Three parts are unvetted: bulk insert, the advisory-lock concurrency case, and that it must be NOT DEFERRABLE.
• To decide — whether to change it at all. "Won't fix, the migration comment is enough" is a legitimate close and should not read as a loose end. Either way the answer belongs in the spec before it belongs in a migration, because id is distinct from new.id is something the spec and practitioner_services: what a practitioner offers, capped at three #90 both reason about.

Full context — the reasoning, the constraints and what has already been ruled out. This is the part a coding agent should read.

practitioner_services_cap is a before insert or update trigger, per docs/spec/profile-and-credentials.md:1710. A before row trigger runs ahead of the policy's WITH CHECK, so a non-owner writing to an approved profile already at three is refused 23514 by the cap rather than 42501 by the policy — a 400 where 403 is the honest answer. It discloses nothing: the count it reflects is already public through an ordinary GET, and on a profile whose services are not public the invoker-scoped count is zero and the policy answers correctly. Found during review of #106; the reasoning is recorded in the migration comment and in this thread.

A constraint trigger fires AFTER, and RLS's WITH CHECK runs before AFTER row triggers, so the ordering corrects itself with no authorization logic — nothing gated on who the caller is, so nothing that can fail open. Probed against the local stack: the stranger's refusal becomes 42501, and the full database suite passes unchanged at 169/169. It also retires id is distinct from new.id rather than working around it, since the row is already in the table by then and the check collapses to count(*) > 3.

That is the reason this is a spec question rather than a fix. id is distinct from new.id is not an implementation detail — the spec reasons about it, #90's body calls it out, and its Done-when list names it as its own assertion ("editing an existing third row in place still succeeds — the assertion that catches the missing id is distinct from new.id"). Retiring it is a design change, so it starts with a spec amendment and the migration follows.

The alternative that was considered and rejected in review: an early return new in the cap, gated on owns_profile(new.practitioner_id) plus a privileged-role allow-list. The allow-list is not optional — without it the cap stops applying to bluehex_admin and #90's "holds against an admin too" assertion goes red. That trades a wrong status code on a public fact for an authorization allow-list inside a function whose whole argument is that it is a rule about what a profile may say rather than an authority over who may say it, and one that fails open on the cap the day it is wrong. Recorded so it is not re-proposed.

Unvetted parts of the AFTER variant

Stated so nobody adopts it off this issue alone. It needs the same mutation testing the current trigger got before it lands.

• The multi-row bulk-insert probe was not re-run against it — the current BEFORE trigger is proven to refuse four rows inserted in one statement.
• The advisory-lock concurrency case was not re-argued for an AFTER trigger.
• It would have to be NOT DEFERRABLE. A DEFERRABLE constraint trigger is a weaker cap than the current one.

Not a bug

Nothing leaks and nothing bypasses. "Won't fix, the migration comment is enough" is a legitimate outcome and this should be closeable that way without it reading as a loose end. What is not acceptable is the question going unrecorded, which is why it is here rather than only in a review thread on a merged PR.

Whichever way it goes, the answer belongs in docs/spec/profile-and-credentials.md before it belongs in a migration.