GPT Review — 23-P3D2 Notification Triggers Prompt rev1

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 rev1

Verdict

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

P3D2 rev1 correctly targets the missing runtime layer: PG triggers that turn comments/drafts/applies into notification events. The scope is right and it addresses per-actor read isolation. However, the prompt has several runtime risks and does not yet fully cover the User’s “summary notification board with who has read what” requirement.

Accepted rev1 points

• Scope limited to 3 trigger functions + 3 triggers.
• No P3D1 table/function changes.
• No Pack 23 function changes.
• Runtime schema inspection is required.
• Comment/draft/version event flows are tested.
• System comment suppression, birth exclusion, actionable filter, per-actor read isolation, self-exclusion, and idempotency are all included.
• notification_runtime=ACTIVE only on PASS.
• Hermes remains blocked pending review.

Required rev2 fixes

P1 — BLOCKER: do not embed dynamic column names through unquoted shell heredoc

Rev1 creates trigger functions using a non-quoted heredoc and shell-substitutes NEW.${COMMENT_ACTOR} etc. This is fragile and unsafe. If a variable is empty or unexpected, SQL corrupts. It also risks shell expansion inside PL/pgSQL.

Patch approach:

• After schema inspection, validate discovered column names against a strict allow-list and non-empty.
• Generate the CREATE FUNCTION SQL through a controlled psql block or shell template that only substitutes validated identifiers.
• Use format('%I', col) pattern if dynamic SQL is needed, or simpler: after validation, write the final static body for the discovered known columns.
• For this runtime, if discovered columns match expected names, hard-code those expected names in the function body.

Minimum acceptable rev2:

• fail if discovered columns are not exactly the expected set used by the trigger body;
• then use a quoted heredoc <<'FNSQL' with static NEW.author_ref, NEW.created_by, etc.

Do not leave “Agent adapt” as an execution-time open instruction inside the CREATE section.

P2 — unit_version may not have a usable actor column

Rev1 requires VERSION_ACTOR and fails if no actor column exists. But earlier reports suggest unit_version may not carry a reviewer actor directly. The actor for version_applied should probably come from the applied draft (unit_edit_draft) or system comment, not guessed.

Patch design in prompt:

• inspect unit_version columns;
• if unit_version.created_by/actor_ref/author_ref exists, use it;
• otherwise, for version_applied, resolve actor via the applied draft:
  • find draft where applied_version_ref = NEW.id, then use draft.created_by or possibly draft.applied_by if exists;
• if no actor can be resolved, use actor_ref='system:apply' and put resolution notes in payload.

Do not make missing version actor a preflight failure.

P3 — Add read-state summary board function or design hook now

User explicitly asked to know on the notification board:

• what changed/commented;
• who has seen it;
• who has not;
• directly from PG.

P3D1 created unread and mark_read only. P3D2 should at least add or explicitly defer a board/read-status function.

Recommended for P3D2 scope: add one read-only helper function:

fn_iu_notification_board(
  p_actor text DEFAULT NULL,
  p_stream text DEFAULT NULL,
  p_limit integer DEFAULT 50
) RETURNS SETOF jsonb

Return per event:

• event_id;
• event_type;
• stream;
• address;
• ref_id;
• actor_ref;
• created_at;
• is_read_for_actor if p_actor provided;
• read_at_for_actor if p_actor provided;
• latest_readers: last 3–5 actors/read_at from iu_notification_read;
• unread_for_actor boolean if p_actor provided;
• next_action/guidance.

If Opus decides not to add this function in P3D2, rev2 must add a hard deferred item P3D3_NOTIFICATION_BOARD_READ_STATUS before Hermes. GPT recommendation: include it in P3D2 if small, because it is read-only and directly satisfies User requirement.

P4 — Trigger functions should be SECURITY DEFINER/search_path or explain why not

Trigger functions run with table owner permissions by default, but for consistency and safety, define:

LANGUAGE plpgsql SECURITY DEFINER SET search_path=pg_catalog,public

Owner should match directus. Revoke EXECUTE from PUBLIC is less relevant for trigger functions but can be done for hygiene. At minimum, verify owner/search_path/prosecdef in tests.

P5 — P3D2 should verify no direct writes except to iu_notification_event

T14 checks no update/delete IU/UV and no canonical_writer, but trigger functions are supposed to INSERT INTO iu_notification_event. Add a source check that the only INSERT target is iu_notification_event, not iu_notification_read or Pack 23 tables.

P6 — Preflight must verify P3D1 constraints/indexes, not only tables/functions

Rev1 assumes P3D1 scaffold is healthy if tables/functions exist. Add checks for:

• uq_notif_event_type_ref partial unique index;
• uq_notif_read_event_actor;
• chk_notif_event_type_stream;
• fk_notif_read_event CASCADE.

If missing, STOP.

P7 — T5 comment test should capture and verify comment ref_id

Rev1 only checks count + stream/source. It should verify ref_id equals the new comment id.

Patch:

• capture result/comment id from fn_iu_comment if returned;
• if fn_iu_comment does not return comment id, query latest comment id for unit/actor/body/time window;
• verify notification ref_id = comment_id.

P8 — T6 system comment suppression should be robust

Rev1 assumes apply creates a system comment and checks comment count unchanged. Good, but add direct verification:

• count system comments created by apply if possible;
• verify those system comment ids do not appear as comment_added notification refs.

If runtime does not expose comment kind enough to identify system comments, report T6_SYSTEM_COMMENT_IDS=NOT_AVAILABLE but still require no comment notification delta during apply.

P9 — T11 mark-read query may call fn_iu_unread twice in one statement

Rev1 uses fn_iu_unread('gpt') inside a CTE and repeatedly references (fn_iu_unread)->> syntax. This can be fragile for set-returning functions.

Patch with explicit alias:

WITH unread AS (
  SELECT j FROM fn_iu_unread('gpt') AS j
  WHERE j->>'address' = :addr
), ids AS (...)
SELECT fn_iu_mark_read(...)

Do the same for count queries. Avoid relying on implicit column name fn_iu_unread.

P10 — T10 actionable filter should use explicit alias too

Same issue as P9. Patch:

SELECT count(*)
FROM fn_iu_unread('gpt','review') AS u(j)
WHERE j->>'ref_id' = '$TEST_DRAFT_ID';

P11 — P3D2 test notification cleanup should verify read rows and event rows are gone

Rev1 deletes both but verifies only event rows. Add read-row verification:

• before cleanup count read rows linked to test events;
• after cleanup both event and linked read rows = 0.

If cleanup fails, phase FAIL/BLOCKED.

P12 — On FAIL cleanup should verify triggers/functions removed

Rev1 drops triggers/functions but does not verify. Add post-cleanup verification:

• zero trg_aa_iu_notif_% triggers;
• zero fn_iu_notif_% functions;
• if not zero, PHASE_STATUS=CRITICAL.

P13 — Final report should include event board/read-state evidence

If P3 adds fn_iu_notification_board, report:

• board sample for gpt before/after mark_read;
• latest_readers includes gpt after mark_read;
• Opus/unread state remains independent.

If deferred, report:

notification_board_read_status=DEFERRED_P3D3

P14 — Cleanup should not delete real events outside test address

Rev1 cleanup uses canonical_address='$TEST_ADDR', good. Preserve this. Also report:

cleanup_scope=canonical_address_only

P15 — Include context pack update after P3D2 PASS or explicitly defer

After P3D2, AI-facing context should mention:

• check inbox: fn_iu_unread(actor);
• mark seen: fn_iu_mark_read(event_ids, actor);
• if board function exists: fn_iu_notification_board(actor).

Either update context pack in P3D2 or add next_required_pack=P3D3_CONTEXT_UPDATE_AND_BOARD.

GPT recommendation: if adding board function, also update context pack in P3D2 or immediately after. But do not overpack if it risks trigger deployment.

Directive to Opus

Patch P3D2 prompt to rev2 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 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.

Recommendation on User’s read-status requirement

The requirement is important enough to satisfy now if possible. A read-only board function is the cleanest PG-native answer:

• fn_iu_unread(actor) = my actionable inbox;
• fn_iu_mark_read(ids, actor) = mark seen;
• fn_iu_notification_board(actor) = summary board showing events + whether this actor read them + latest readers.

This keeps everything PG-first and avoids external code searching manually.

Summary

P3D2 rev1 is directionally right, but rev2 must remove dynamic SQL fragility, handle version actor resolution correctly, add/plan the read-status board, and harden cleanup and tests before trigger deployment.