GPT Review — 23-P3C1 Safe Functions Prompt rev3

Date: 2026-05-07
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3c1-iu-edit-draft-safe-functions-prompt.md rev3

Verdict

Rev3 is close, but NOT ready to dispatch. Rev4 required.

Opus fixed the prior SQL blockers and moved the prompt close to executable quality. The core function design is accepted. Remaining issues are concentrated in the test harness and grant discovery, not the core UX model.

Accepted rev3 improvements

• GUC-based owner passing inside the same psql session fixed the dollar-quote substitution bug.
• fn_iu_comment now uses v_comment_id uuid and RETURNING id INTO v_comment_id.
• Bad context.draft_id UUID now returns JSON instead of throwing raw exception.
• fn_iu_edit_plan supports title and no-change title logic.
• Candidate list includes hash preview and does not leak body.
• Cleanup uses exact signatures and no CASCADE.
• T21 source safety check is present.
• Natural comment behavior remains simple but safe.

Required rev4 fixes

P1 — BLOCKER: T5 creates two drafts by accident

Rev3 runs:

1. T5_JSON=$(SELECT fn_iu_create_edit_draft(...)) — creates draft #1.
2. T5_PARSED=$(WITH r AS (SELECT fn_iu_create_edit_draft(...)) ...) — creates draft #2.

This breaks the rest of the test plan:

• T9 expects exactly one open draft, but there are already two.
• T16 expects DRAFT_BEFORE + 2, but actual will be +3 after T10.

Patch: delete the first T5_JSON / T5_STATUS / DRAFT_A_ID parse block. Keep only one T5 call that both creates and parses the draft.

P2 — TEST_ADDR_B must be selected as an IU with zero open drafts

Rev3 picks the second canonical address, but does not prove it has no open draft. If retained drafts from prior tests exist, T14 may fail or test the wrong condition.

Patch Gate 9:

• select TEST_ADDR_A as an IU with enough normal current version data;
• select TEST_ADDR_B as a different IU where no open draft exists:

SELECT iu.canonical_address
FROM information_unit iu
WHERE iu.canonical_address <> :'addr_a'
  AND NOT EXISTS (
    SELECT 1 FROM unit_edit_draft d
    WHERE d.unit_id = iu.id AND d.draft_status='open'
  )
ORDER BY iu.canonical_address
LIMIT 1;

If no such B exists, STOP with a clear reason. Do not allow T14 to be iu_not_found or ambiguous.

P3 — Grant discovery query should use aclexplode safely, and grants should follow discovered grantees

Rev3 captures FN_GRANTEES using repeated aclexplode(proacl) calls in a query shape that is fragile. It also does not actually use FN_GRANTEES; it grants only to FN_OWNER.

Patch:

Preflight:

SELECT COALESCE(array_to_string(array_agg(DISTINCT x.grantee::regrole::text), ','), '')
FROM pg_proc p
JOIN pg_namespace n ON p.pronamespace=n.oid
LEFT JOIN LATERAL aclexplode(p.proacl) x ON true
WHERE n.nspname='public'
  AND p.proname='fn_iu_create'
  AND x.privilege_type='EXECUTE'
  AND x.grantee <> 0;

Then:

• if FN_GRANTEES is empty, use FN_OWNER;
• pass FN_GRANTEES_EFFECTIVE into the CREATE session as a GUC;
• in the GRANT DO block, loop over string_to_array(current_setting('app.p3c1_grantees'), ',') and grant each role.

This follows Pack 22 privilege pattern instead of assuming owner-only.

P4 — Tests should avoid raw interpolation for addresses where feasible

Rev3 still injects $TEST_ADDR_A / $TEST_ADDR_B directly into SQL string literals. Current canonical addresses are likely safe, but the design principle is address-first and schema evolution may allow wider addresses later.

Patch key tests to pass addresses via psql -v addr_a="$TEST_ADDR_A" -v addr_b="$TEST_ADDR_B" and use :'addr_a' inside SQL. At minimum, do this for tests that use CTEs and body text.

This is not as severe as P1, but it prevents quote/newline surprises.

P5 — T19 should verify search_path, not only SECURITY DEFINER and PUBLIC

Security requirement includes SET search_path=pg_catalog,public.

Patch T19 to check proconfig includes the search path for all 4 functions.

Example:

SELECT count(*)
FROM pg_proc
WHERE oid IN (...)
  AND prosecdef = true
  AND proconfig::text LIKE '%search_path=pg_catalog, public%';

Expected 4, or compare more robustly by unnesting proconfig.

P6 — Verify intended grantees can execute, not only PUBLIC absent

After P3, T19 should verify:

• PUBLIC has no EXECUTE;
• each effective intended grantee has EXECUTE on all 4 functions.

This prevents a PASS where functions exist but nobody intended can call them.

P7 — Final report variables may be unset on early failure

If preflight fails before tests, variables such as IU_NOW, UV_NOW, DRAFT_NOW, COMMENT_NOW, DRAFT_A_ID, DRAFT_B_ID, and UV_LC may be unset in the final report.

Initialize them in setup or compute final counts unconditionally before echo.

Recommended setup defaults:

IU_NOW=""; UV_NOW=""; DRAFT_NOW=""; COMMENT_NOW=""
DRAFT_A_ID=""; DRAFT_B_ID=""; UV_LC=""
DROP_FAIL="0"; CLEANUP_STATUS="NOT_RUN"

P8 — Cleanup failure should prevent final PASS even if PHASE_STATUS was not set

Rev3 sets PHASE_STATUS="CRITICAL" if drop fails, but final logic can still override in some branches. Make final logic preserve CRITICAL:

if [ "$PHASE_STATUS" = "CRITICAL" ]; then
  P3C2_READINESS="BLOCKED"
elif ...

P9 — Explicitly mark retained test rows as expected on PASS

P3C1 creates draft/comment rows and retains them on PASS. This is acceptable, but the report should say:

• test_rows_retained_on_pass=true
• list DRAFT_A_ID, DRAFT_B_ID
• explain they are retained for P3C2 / audit testing, not cleanup candidates.

P10 — Add a concise README/usage output section for AI UX

Rev3 has UX samples, but add a final report section:

AI-facing 3-line interface

Expected:

SELECT fn_iu_edit_plan(address, body, actor);
SELECT fn_iu_create_edit_draft(address, body, actor, reason, title);
SELECT fn_iu_comment(address, actor, comment);

Mention that fn_iu_edit / apply comes in P3C2. This reinforces the Apple/iPhone principle and avoids Agent confusion after P3C1.

Directive to Opus

Patch P3C1 prompt to rev4 with P1–P10.

Path:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3c1-iu-edit-draft-safe-functions-prompt.md

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

Hard boundaries remain

• No dispatch.
• No table DDL.
• No schema changes.
• No trigger changes.
• No gateway changes.
• No IU/UV writes.
• No vector mutation.
• No cleanup.
• No Pack 2C.

Summary

Rev3 fixed the major SQL body issues. The main remaining blocker is the test harness accidentally creating two drafts in T5, which invalidates the safety tests. Rev4 should fix that, strengthen address/test isolation, and ensure grants/search_path are verified exactly.