GPT Review — 23-P3D2 Notification Triggers Prompt rev2

Date: 2026-05-08
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d2-notification-triggers-prompt.md rev2

Verdict

Direction accepted, but do not dispatch yet. Rev3 required.

Rev2 improves the design substantially and adds the read-status board requested by the User. However, several execution blockers remain. Most importantly, the prompt still uses shell interpolation inside the CREATE FUNCTION heredoc despite claiming static columns, and it checks a partial unique index as if it were a pg_constraint.

Accepted rev2 improvements

fn_iu_notification_board was added to satisfy the “who has read what” board requirement.
Version actor fallback is considered.
Trigger functions are SECURITY DEFINER with pinned search_path.
P3D1 scaffold checks were expanded.
Tests now use explicit SRF aliases for fn_iu_unread.
Cleanup scope is limited by canonical address.
System comment suppression, birth exclusion, actionable filter, per-actor read isolation, self-exclusion, idempotency, and source safety are still covered.

Required rev3 fixes

P1 — BLOCKER: remove shell interpolation from CREATE FUNCTION heredoc

Rev2 still uses:

"${PSQL[@]}" -v fn_owner="$FN_OWNER" <<FNSQL
...
NEW.${COMMENT_ACTOR}
NEW.${DRAFT_ACTOR}
${V_ACTOR_EXPR}
...
FNSQL

This is not static and remains fragile. It can corrupt SQL if a variable is empty or unexpected.

Patch:

Preflight must validate exact expected columns for this runtime:
- unit_edit_comment.author_ref
- unit_edit_comment.comment_kind
- unit_edit_comment.author_type if present and used for system suppression
- unit_edit_draft.created_by
- unit_edit_draft.canonical_address
- unit_edit_draft.applied_by if unit_version has no actor
If these expected columns are not present, STOP and report schema mismatch. Do not ask Agent to substitute inside CREATE.
Use a quoted heredoc <<'FNSQL' with hard-coded static column names.
For unit_version actor, use one of two static quoted CREATE blocks:
- block A if unit_version.created_by exists;
- block B fallback via unit_edit_draft.applied_by / system:apply.

Do not leave “Agent adapt” instructions inside CREATE section.

P2 — P3D1 partial unique index check is wrong

Rev2 checks uq_notif_event_type_ref in pg_constraint, but it is a partial unique index, not a constraint.

Patch preflight:

verify uq_notif_event_type_ref via pg_class/pg_index and require indisunique=true and indpred IS NOT NULL;
verify uq_notif_read_event_actor via pg_class/pg_index or pg_constraint depending how P3D1 created it;
keep chk_notif_event_type_stream and fk_notif_read_event in pg_constraint.

P3 — Owner assignment syntax is unsafe

Rev2 uses:

ALTER FUNCTION ... OWNER TO :'fn_owner';

:'fn_owner' is a SQL string literal, not necessarily a valid role identifier for OWNER TO.

Patch with a DO block using format('%I', current_setting(...)), or psql identifier quoting :"fn_owner" if reliable. Preferred pattern:

SELECT set_config('app.p3d2_owner', :'fn_owner', true);
DO $$
DECLARE v_owner text := current_setting('app.p3d2_owner');
BEGIN
  EXECUTE format('ALTER FUNCTION public.fn_iu_notif_comment() OWNER TO %I', v_owner);
  ...
END $$;

P4 — Revoke PUBLIC on trigger functions too

Rev2 revokes PUBLIC only on fn_iu_notification_board. For hygiene and consistency, revoke PUBLIC from all four new functions:

fn_iu_notif_comment()
fn_iu_notif_draft()
fn_iu_notif_version()
fn_iu_notification_board(text,text,integer)

Then grant only intended grantees if needed. Trigger functions do not need public execute.

T15 should verify PUBLIC execute = 0 for all four functions.

P5 — System comment suppression should inspect `author_type` if present

The User requirement is operationally “comment freely, but do not duplicate system/apply comments as comment notifications.” Rev2 only checks comment_kind='system'.

Patch:

preflight inspect whether unit_edit_comment.author_type exists;
if exists, trigger suppresses when comment_kind='system' OR author_type='system';
if not exists, suppress by comment_kind='system' and report comment_author_type=ABSENT.

T6 should report system comment ids if detectable and verify they do not appear as comment_added refs.

P6 — Board function must include `unread_for_actor`

User asked for a board showing who has read/has not read. Rev2 returns is_read_for_actor and latest_readers, but does not include unread_for_actor.

Patch board return JSON:

unread_for_actor: if p_actor is provided, boolean inverse of read state, while respecting actionable filter for draft events;
keep is_read_for_actor and read_at_for_actor.

P7 — Board function should validate stream and limit

Add minimal validation:

p_stream IS NULL OR p_stream IN ('comment','review','update');
limit clamp 1..500 already exists, keep it.

For invalid stream, either return no rows with a clear JSON error is hard for SETOF; simpler acceptable behavior: raise a controlled exception or return an empty set and document. GPT preference: return empty set is acceptable if report says so, but test it.

P8 — T16 board test should prove latest_readers includes GPT after mark_read

Rev2 only checks board_count >= 1. Strengthen T16:

after T11 marks events read for GPT, board for GPT should include at least one event with is_read_for_actor=true;
latest_readers JSON should contain actor gpt;
unread_for_actor=false for the marked event.

P9 — Source safety must verify INSERT target whitelist

Rev2 only forbids INSERT into iu_notification_read; it should assert trigger functions insert into iu_notification_event and not into Pack 23 tables.

Patch T14 forbidden patterns to include:

insert into information_unit
insert into unit_version
insert into unit_edit_comment
insert into unit_edit_draft
insert into iu_notification_read
update/delete information_unit|unit_version|unit_edit_comment|unit_edit_draft
app.canonical_writer

Also require each trigger function source contains insert into public.iu_notification_event or insert into iu_notification_event.

P10 — Cleanup read-row verification is misleading after deleting events

Rev2 verifies read rows with:

SELECT count(*) FROM iu_notification_read
WHERE event_id IN (SELECT id FROM iu_notification_event WHERE canonical_address='$TEST_ADDR')

after event rows have already been deleted. This always returns 0 because the subquery is empty.

Patch cleanup:

capture test event ids before deleting events, e.g. into a temp table or array;
delete read rows for those ids;
delete events;
verify no read rows remain for captured ids;
verify no event rows remain for test address.

P11 — Failure cleanup must not accidentally remove future board function if name pattern changes

Rev2 counts remaining functions with proname LIKE 'fn_iu_notif%', which includes board currently. Fine. But make the cleanup list explicit:

fn_iu_notif_comment
fn_iu_notif_draft
fn_iu_notif_version
fn_iu_notification_board

Verify exact remaining count for these names = 0.

P12 — Add context pack update decision now

Rev2 says context update deferred or done post-PASS, but final report does not make this a hard next step.

Patch report field:

next_required_pack=P3D3_CONTEXT_UPDATE_FOR_NOTIFICATION_COMMANDS

unless Opus chooses to include context update in P3D2. GPT recommends defer context update to P3D3 to keep trigger deployment focused.

P3D3 should add commands:

fn_iu_unread(actor)
fn_iu_mark_read(event_ids, actor)
fn_iu_notification_board(actor)

P13 — Add summary-board requirement to final report

Final report should include:

notification_board=ACTIVE
board_function=fn_iu_notification_board(text,text,integer)
per_actor_read_state=ACTIVE

on PASS.

P14 — P3D2 should report trigger function security metadata

Final report should include:

owner for each new function;
SECURITY DEFINER status;
search_path;
PUBLIC execute absent.

P15 — If create block fails after partial trigger creation, cleanup must run

Rev2 sets FN_STATUS=FAIL, but fail cleanup only runs if FN_STATUS=OK.

If CREATE partially succeeds and then fails before COMMIT, transaction should rollback. But if any object remains, prompt should verify. Patch:

after FN_STATUS=FAIL, check for any P3D2 trigger/function objects;
if any exist, attempt cleanup;
if cleanup leaves any, PHASE_STATUS=CRITICAL.

Directive to Opus

Patch P3D2 prompt to rev3 with P1–P15.

Path:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d2-notification-triggers-prompt.md

Do not dispatch after patch. Return for GPT/User final review.

Hard boundaries remain

No dispatch.
No Pack 23 function changes.
No P3D1 table changes.
No gateway/birth trigger changes.
No vector mutation.
No LISTEN/NOTIFY.
No retention/archival.
No external queue.
No global read flag.
No Hermes production start.

Summary

Rev2 adds the right board function and broadens coverage, but it still has execution hazards. Rev3 must make CREATE static and quoted, fix the partial-index preflight, enforce PUBLIC revokes, add unread_for_actor, and make cleanup verification honest before dispatch.