KB-761E

GPT Review — 22-P3-P2 Trigger Guard Prompt rev1

9 min read Revision 1
gpt-reviewpack-22p3p2trigger-guardrev2-required

GPT Review — 22-P3-P2 Trigger Guard Prompt rev1

Date: 2026-05-06
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/22-p3-p2-iu-gateway-trigger-guard-prompt.md

Verdict

Do not dispatch rev1. Rev2 required.

Opus did the right thing by drafting compactly and including most P3-P1 lessons from the start. However, P3-P2 is an enforcement pack. There are several correctness issues that must be fixed before execution.

To reduce cycles, this review gives copy-ready patches. Opus should patch exactly these and avoid expanding scope.

Must-fix items

P1 — SECURITY INVOKER guard likely blocks Directus because directus loses dot_config read? Verify or switch to DEFINER

The guard function is SECURITY INVOKER and reads public.dot_config. If future callers are roles without read on dot_config, the guard may fail with permission error instead of returning the intended README/canonical-path message.

Since this trigger runs on every IU/UV write, prefer predictable behavior:

CREATE FUNCTION public.fn_iu_gateway_write_guard()
RETURNS trigger
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path=pg_catalog,public
AS $$ ... $$;

Then immediately machine-check:

  • PUBLIC EXECUTE absent or irrelevant for trigger functions;
  • owner/security/search_path/provolatile are expected.

If Opus wants to keep INVOKER, the prompt must verify every potential table writer role has dot_config read. That is slower/riskier. For rev2: use SECURITY DEFINER with fixed search_path.

P2 — Trigger guard will block the updated_at trigger if it UPDATEs the same table? Inspect assumption

P3-P2 creates BEFORE UPDATE guard. IU already has fn_iu_updated_at BEFORE UPDATE. If fn_iu_updated_at only mutates NEW fields inside the trigger, fine. If it issues an UPDATE statement, the guard may recurse/block.

Add preflight:

SELECT p.proname, left(p.prosrc, 1000) AS source_preview
FROM pg_trigger t
JOIN pg_proc p ON p.oid=t.tgfoid
WHERE t.tgrelid='public.information_unit'::regclass
  AND p.proname='fn_iu_updated_at';

Agent must confirm from source preview: it modifies NEW only or otherwise flag risk. No need for long review; just record.

P3 — Direct INSERT/UPDATE block tests inside same transaction after canonical create may inherit marker and falsely pass

This is the most important bug.

fn_iu_create sets set_config(..., true), transaction-local. In P3-P2 rev1, all tests run inside one explicit transaction after canonical create. Therefore the marker may remain set for the rest of that transaction. Direct INSERT/UPDATE tests may be allowed, not blocked, or test results may not reflect real outside-canonical behavior.

Patch test design:

  • Main transaction creates guard function/triggers and commits after basic deployment checks.
  • Then run canonical create in a separate psql call/session.
  • Then run direct block tests in separate psql calls/sessions without marker.

Alternative: after canonical create in same transaction, explicitly RESET app.canonical_writer before direct tests. But post-commit separate sessions are clearer and closer to real wrong-door behavior.

For rev2, use separate sessions:

  1. TX1: create guard function + triggers, COMMIT.
  2. POST1: canonical create via fn_iu_create, verify PASS.
  3. POST2: direct IU INSERT blocked.
  4. POST3: direct UV INSERT blocked.
  5. POST4: direct IU UPDATE blocked.
  6. POST5: direct UV UPDATE blocked.
  7. POST6: marker leak + trigger count + gateway mode.

This also makes rollback behavior different: if TX1 commits but tests fail, report state and do not auto-drop triggers unless prompt explicitly includes rollback. That is acceptable if final verdict is FAIL/BLOCKED.

P4 — Direct UV INSERT test may fail on FK before guard if trigger timing/order surprises

Rev1 assumes guard fires before FK. BEFORE trigger should fire before FK, but to make the test robust, use a valid existing IU id from the pilot or existing row, not a random UUID. That ensures if guard does not fire, the INSERT could proceed far enough, making test meaningful.

Suggested test:

INSERT INTO public.unit_version(id, unit_id, body, content_hash, version_seq, created_by)
SELECT gen_random_uuid(), iu.id, 'direct', 'direct', 999, 'direct_test'
FROM public.information_unit iu
WHERE iu.canonical_address = :'pilot_addr';

Guard should block. If it does not, the row may insert; test must then detect and fail. Use a unique version_seq=999 and verify no row exists after caught block.

P5 — Direct UPDATE tests must avoid running through marker in same session

Covered by P3. Ensure UPDATE tests are separate psql calls/sessions or reset marker. Prefer separate sessions.

P6 — Error-message tests should check readme/canonical content, not only %IU Gateway%

The purpose is to guide wrong callers. Tests should assert the exception contains:

  • IU Gateway blocked;
  • fn_iu_create or the canonical function from dot_config;
  • README or the configured readme path.

Keep simple with position() or LIKE checks.

P7 — gateway_mode=enforced update should happen only after all block tests pass

Rev1 updates dot_config to enforced before COMMIT in the same transaction as guard creation, before post-commit separate tests. If later direct-block tests fail, mode still says enforced.

Patch:

  • TX1 creates guard function/triggers but keep mode prepared or set deploying.
  • After all post-commit tests pass, run a final small transaction to set iu_create.gateway.mode='enforced'.
  • Final verdict requires GW_MODE=enforced.

If final mode update fails, phase FAIL/BLOCKED.

P8 — Final verdict must include gateway mode check

Rev1 prints gateway_mode but final condition does not require it.

Patch final condition to require:

[ "$GW_MODE" = "enforced" ]

P9 — If guard function/trigger already exists, verify-idempotent or STOP; current STOP okay but report should include state

Current early stop is fine. Add to early_stop output if available:

  • guard_fn_exists
  • guard_trg_exists

This helps review if rerun happens.

P10 — Function/trigger creation should include post-create metadata assertions

After TX1 commit, verify:

  • to_regprocedure('public.fn_iu_gateway_write_guard()') exists;
  • function is SECURITY DEFINER, search_path=pg_catalog,public;
  • exact two triggers exist with event bits for INSERT and UPDATE;
  • trigger enabled state is ordinary (tgenabled='O').

Do not rely only on count + grep.

Use shell status variables and separate SQL calls:

1. Preflight: marker, keys, no existing guard, trigger baseline.
2. TX1: BEGIN; CREATE guard function; CREATE two triggers; COMMIT.
3. POST_DEPLOY: verify function/triggers metadata.
4. CANONICAL_TEST: fn_iu_create pilot in fresh session.
5. DIRECT_TESTS: four separate tests in fresh sessions or one fresh session with no marker.
6. MODE_UPDATE: if all tests PASS, update dot_config gateway.mode=enforced.
7. FINAL_VERIFY: mode, marker leak, pilot invariants, trigger counts.
8. Final machine verdict.

Directive to Opus

Patch P3-P2 prompt to rev2 with P1–P10. Keep compact; do not add new architecture.

Most important: do not run direct block tests in the same transaction after canonical create, because set_config(..., true) transaction-local marker can contaminate the tests.

Do not dispatch after patch; return for GPT/User review.

Hard boundaries remain

  • no GRANT/REVOKE;
  • no role separation;
  • no Directus permission changes;
  • no detector implementation;
  • no DOT registration;
  • no adapter implementation;
  • no cleanup pilots;
  • no Pack 2C.

Summary

Rev1 is close but test sequencing is wrong. The canonical marker is transaction-local; therefore direct-block tests must run without inheriting it. Rev2 should split deployment, canonical test, direct-block tests, mode update, and final verification into separate sessions/steps.

Back to Knowledge Hub knowledge/dev/laws/dieu44-trien-khai/reviews/gpt-review-22-p3-p2-trigger-guard-prompt-rev1-2026-05-06.md