KB-6522 rev 3

Codex Static Review — PIDX v0.3.1

18 min read Revision 3
workflow-managepidxv0.3.1static-reviewcodex2026-06-23

Codex Static Review — PIDX v0.3.1

1. Executive verdict

PASS_WITH_CAVEATS_REQUIRES_PATCH

The v0.3.1 B1 patch is correct: all three fingerprint expressions now cast pg_class.relkind to text, and no remaining executable text || c.relkind ambiguity exists.

The core readiness design is materially stronger than v0.2. H1 and H5 are closed, the 23 requested false-READY attacks do not expose a remaining green path, and scope remains 2T2V.

The package is not ready for Owner build authorization discussion because two HIGH safety claims are not enforced:

  1. the idempotency fingerprint can classify materially drifted constraints, indexes, view security, ownership, and grants as PRESENT_MATCHING;
  2. rollback trusts writer-controlled source_ref values and a free-form custom GUC as “backup proof”, so real rows can be dropped without a verified backup.

One additional MEDIUM access defect remains: table-level INSERT allows pidx_writer to supply an explicit GENERATED ALWAYS identity by using OVERRIDING SYSTEM VALUE; there is no separate “OVERRIDING privilege” to withhold.

Static review only: no DDL/DML, CREATE, build, dry-run, role/grant change, or PG/Directus/DOT/runtime/source-design mutation was performed.

2. Files read

Rules and foundations, read from Agent Data in the main process:

  • .claude/skills/incomex-rules.md (local, full 36-rule/8-step skill);
  • knowledge/dev/ssot/operating-rules.md v7.58 rev 51;
  • knowledge/dev/ssot/vps/vps-operating-rules.md v1.0 rev 2;
  • knowledge/dev/laws/constitution.md current v4.6.3 rev 44 (the requested v4.0 lineage);
  • knowledge/dev/laws/law-01-foundation-principles.md v3.3;
  • knowledge/dev/laws/dieu20-thiet-ke-truoc-trien-khai.md;
  • knowledge/dev/laws/dieu33-postgresql-law.md v2.1;
  • knowledge/dev/laws/law-08-dependency.md;
  • knowledge/dev/laws-new/workflow-manage/de-bai-quan-ly-quy-trinh.md rev 21.

Required package:

  • design/pidx-codex-review-packet-v0.3.1.md;
  • design/pidx-ddl-candidate-v0.3.1.sql.md — all 1,031 lines;
  • all five v0.3.1 delta stubs;
  • inherited full v0.3 build design, readiness logic, seed slice, test plan, and build/access/idempotency contract;
  • reports/pidx-v0.3-brutal-self-audit-2026-06-23.md;
  • reports/pidx-v0.3.1-patch-from-self-audit-2026-06-23.md;
  • reports/pidx-v0.3.1-brutal-self-audit-2026-06-23.md;
  • reports/pidx-build-design-go-no-go-v0.3.1.md;
  • both required Codex v0.2 reports.

At review time, Agent Data's knowledge/dev/laws-new/workflow-manage/ listing ended at v0.2. The v0.3/v0.3.1 package was therefore read from the exact workspace paths named by the mission. This provenance limitation does not change the SQL findings but must be resolved when publishing the patched package.

PostgreSQL semantics were cross-checked against official PostgreSQL 16 documentation for INSERT privileges and OVERRIDING SYSTEM VALUE, security_invoker views, system information functions, and custom two-part settings.

3. v0.3.1 patch verification

B1 relkind::text

RESOLVED.

Exact executable sites:

  • §7.1 build-state verifier: line 714;
  • §10.1 P-5 pre-rollback fingerprint: line 925;
  • §10.3(b) post-rollback fingerprint: line 1008.

Each uses c.relkind::text. A negative search found no executable || c.relkind without that cast. The remaining uncast occurrence is explanatory prose describing the old failure.

No new cast ambiguity was introduced. pg_get_viewdef(c.oid) is wrapped in COALESCE(..., ''); the package's SELECT-only live evidence reports an empty result for base tables, so mixed table/view input does not break the expression.

Fingerprint meaning

NOT SUFFICIENT for PRESENT_MATCHING.

The hash covers relation name, relkind, visible column name/type/not-null signatures, and view definition. It does not bind:

  • defaults, generated/identity attributes or identity mode;
  • CHECK, UNIQUE, PK, or FK definitions/actions;
  • the four explicit indexes;
  • reloptions, including security_invoker=true;
  • object owner;
  • table/column/view grants and role membership.

Therefore a build can lose the status CHECK, change FK behavior, drop a hot-path index, remove security_invoker, or change writer restrictions and still return PRESENT_MATCHING. Because that state is defined as no-op success, this is HIGH H4 drift acceptance.

The stated first-build capture also needs tightening: the canonical expected value must be derived from the reviewed artifact or accompanied by the full canonical verifier, not learned from an arbitrary first resulting state.

4. Prior HIGH resolution review

Group Verdict Evidence
H1 lifecycle/usability RESOLVED top-level non-active status gates before green; required lifecycle-bearing sources require usable=true; unknown usability blocks; optional failure warns; bare-label usability is aggregate/deterministic.
H2 identity/delete PARTIAL — MEDIUM no writer DELETE, no UPDATE(procedure_code), and FK RESTRICT/RESTRICT are correct. But table-level INSERT includes id; PostgreSQL accepts an explicit GENERATED ALWAYS identity when the INSERT uses OVERRIDING SYSTEM VALUE. The design's claim that this capability is “not granted” is incorrect.
H3 rollback NOT RESOLVED — HIGH Guard B raises for a non-seed marker, but the writer can insert/update source_ref to the seed literal. A free-form custom GUC is accepted as backup confirmation without checking that archive relations exist or match row counts/content.
H4 access/idempotency NOT RESOLVED — HIGH roles/grants and security_invoker are concrete, and the absence/collision gate is fail-closed. The matching verifier omits safety-critical catalog state, so drift can be accepted as matching.
H5 tests RESOLVED for readiness; PARTIAL for safety 18-row totals are internally consistent; T1–T25/S1–S10 cover readiness without >=0. Safety tests omit explicit-identity INSERT, seed-marker tampering, fake-backup GUC, generic pg_depend in-transaction coverage, and drift in constraints/indexes/grants/reloptions.

5. SQL static review

The two table definitions, four indexes, both view bodies, grants, and DO-block syntax are statically coherent for PostgreSQL 16. The strict parser, CTE dependencies, bool_and/bool_or label aggregate, explicit LATERAL unnest(... AS u(w)), JSONB aggregation, privilege-function OID forms, warning rollup, and readiness precedence are plausible and internally consistent.

Official PostgreSQL 16 documentation confirms security_invoker is a supported view option and that its base-relation privileges are checked against the view user. The design correctly notes that the consumer also needs direct source-table privileges. The final verifier should test those source grants under the actual reader role, not only view SELECT.

Findings:

  • HIGH — incomplete matching fingerprint: §7.1 cannot prove the reviewed artifact is present; see §3.
  • HIGH — unverified rollback authorization: §10.2 treats a string assertion as a backup artifact; see §7.
  • MEDIUM — identity INSERT bypass: §8's table-level INSERT plus OVERRIDING SYSTEM VALUE contradicts the “uneditable identity” claim.
  • MEDIUM — Guard A does not re-run P-2c: §10.2 claims to recheck P-2a/b/c in-transaction but its DO block checks only rewrite dependents and inbound FKs. Default DROP ... RESTRICT is a final fail-closed backstop, so this is not HIGH, but the guard/evidence claim is false.
  • MEDIUM — reader contract is not self-contained: pidx_reader receives the four PIDX SELECT grants but not the source-registry grants required by security-invoker views. Existing direct privileges of context_pack_readonly may make the prototype work, but the role contract and its verifier must enumerate/verify the full source privilege set.
  • LOW — apply assembly remains a recipe: §7.2 contains the gate plus comments telling a future runner to concatenate sections. Before authorization, the registered migration DOT must contain one immutable, reviewable apply artifact and paired verifier.

No DDL/DML was run to parse CREATE wrappers. The package's reported SELECT-only resolver/inventory-body execution is useful evidence but is not independent Codex production evidence.

6. False READY risk review

No remaining HIGH/BLOCK false-READY route was found.

# Attack Static result
1–2 draft / retired procedure NOT_READY + PROCEDURE_NOT_ACTIVE.
3–4 inactive / unknown / NULL top-level status CHECK+NOT NULL prevent them; defensive non-active branch cannot green if constraint later changes.
5–6 zero required / optional-only UNMAPPED.
7 declared ready vs computed not ready no green influence; readiness_drift=true.
8 prefix mismatch INVALID_REF.
9–11 extra, missing, empty segment strict arity/empty checks → INVALID_REF.
12 invalid kind table CHECK prevents it; parser ELSE is false.
13 malformed label strict one/two-segment parsing; missing/invalid cannot green.
14 bare multi-match label warning; mixed/unknown lifecycle blocks; all-usable ambiguous case can only be READY_WITH_WARNINGS.
15 logical-only collection physical missing → MISSING + mismatch warning.
16 physical-only collection EXISTS plus mismatch warning → at most READY_WITH_WARNINGS.
17 required status NULL/unknown unsatisfied + SOURCE_USABILITY_UNKNOWN.
18 required inactive/retired/deprecated unsatisfied + SOURCE_NOT_USABLE.
19 unimplemented approval handler required source unsatisfied + handler-specific warning.
20 overloaded function EXISTS with OVERLOADED_FUNCTION; at most READY_WITH_WARNINGS.
21 unreadable source READ_BLOCKED precedes EXISTS/MISSING.
22 required UNKNOWN_SOURCE unsatisfied → NOT_READY.
23 warning status with empty flags rollup derives RWW only from non-empty aggregated flags; S2 asserts the invariant.

Residual false-truth risk, not false-green: source registry codes probed with LIMIT 1 assume uniqueness for DOT/approval/workflow codes. The build verifier should prove the relevant uniqueness constraints or multiplicity invariants.

7. Rollback/access/idempotency review

Rollback

No CASCADE, safe drop order, explicit provenance columns, and transaction-local RAISE guards are improvements.

The current data guard is bypassable by normal supported operations:

  • pidx_writer has INSERT on all parent columns and INSERT/UPDATE on all ingredient columns;
  • it also has UPDATE(source_ref) on the parent;
  • any real declaration can therefore carry or be changed to the literal seed marker;
  • Guard B then counts it as seed and allows destruction with no backup;
  • for rows it does count as non-seed, any non-empty pidx.rollback_backup_ref passes. PostgreSQL accepts arbitrary two-part custom settings, so the string does not prove an archive exists.

Required behavior: either abort rollback whenever either table contains rows unless a verified archive manifest exists, or make provenance immutable/system-authored and validate both archive relations, expected schema, row counts, and checksums before DROP.

Access

Natural-key update and parent DELETE are correctly denied to pidx_writer; RESTRICT/RESTRICT conserves child dependencies. Replace table-level INSERT with column-level INSERT excluding id (and consider excluding system provenance), then add a negative OVERRIDING SYSTEM VALUE test.

Idempotency

ABSENT and partial/collision behavior is fail-closed. PRESENT_MATCHING is not trustworthy until the verifier binds all safety-critical DDL and ACL state. Re-run the full canonical verifier on every no-op path.

8. Scope/no-new-truth review

PASS.

The package remains exactly:

  • pidx_procedure;
  • pidx_procedure_ingredient;
  • v_pidx_inventory_current;
  • v_pidx_procedure_readiness.

It adds no link table, engine, scheduler, event bus, materialized view, RAG/vector, pg_trgm, Nuxt logic, governance/KG/birth dependency, auto-execution, auto-fix, auto-approval, auto-DOT registration, or PIDX-owned classification enrichment. Manifest, notes, declared maturity, and seed fixtures do not set READY. PIDX remains the eye, not the hand.

The mutable provenance marker is not a readiness source of truth, but it currently acts as destructive rollback authorization; that role must be removed or governed as described in §7.

9. Test plan review

The declared totals match the 18 seed rows:

READY=4, READY_WITH_WARNINGS=2, NOT_READY=10, UNMAPPED=2, drift=3.

T1–T25 and S1–S10 cover all readiness statuses, parser cases, lifecycle gates, bare labels, collection mismatch, warning completeness, inventory deduplication, and narrowness. There is no weak >=0 proof.

Required safety additions:

  1. mutate each omitted fingerprint dimension (CHECK/FK/index/security_invoker/grant/owner/identity mode) and require PARTIAL_OR_DRIFTED;
  2. attempt explicit parent and ingredient IDs with OVERRIDING SYSTEM VALUE as pidx_writer and require denial;
  3. create a non-seed row, retag it with the seed literal, and prove rollback still aborts;
  4. set a fake non-empty backup GUC with no archive and prove rollback aborts;
  5. corrupt archive row count/checksum and prove rollback aborts;
  6. add a non-rewrite, non-FK normal dependency and prove the in-transaction generic dependency guard catches it;
  7. execute view/source readability assertions under the actual mapped reader role.

10. Owner-decision items before build

  1. Điều 33 classification: PG-native technical index or governed Directus entity.
  2. PG-native prototype access versus Directus registration/permissions.
  3. Governed build route: registered migration DOT + paired verifier, authorized by APR, or another approved route.
  4. Role provisioning/mapping for owner, writer, reader, and context_pack_readonly.
  5. Canonical expected-definition strategy: artifact-derived hash plus complete catalog/ACL assertions.
  6. Backup destination and machine-verifiable archive manifest policy.
  7. Whether the 13 base / 18 total fixtures persist or are build-test-only.
  8. Acceptance rule for io/checker/template/report = UNKNOWN_SOURCE.
  9. Whether PIDX's surrogate IDs must be system-generated only; recommendation: yes.
  10. Whether provenance is immutable system metadata or rollback ignores provenance and requires a verified backup for any rows.

These are decisions only; this report does not request authorization.

11. Required patches

BLOCK: none.

HIGH:

  1. Expand the matching verifier to bind constraints, identity/default attributes, indexes, reloptions, ownership, grants, and required role membership/source access. Any mismatch must classify PARTIAL_OR_DRIFTED; the no-op path must run the complete verifier.
  2. Replace the writer-controlled seed-marker/free-form-GUC rollback exception with machine-verifiable data protection. Rollback must not DROP populated tables unless a checked archive/backup manifest proves both tables are recoverable.

MEDIUM:

  1. Use column-level INSERT grants excluding id and add an explicit OVERRIDING SYSTEM VALUE denial test.
  2. Remove writer mutation of system provenance, or remove provenance from rollback authorization.
  3. Re-run the generic P-2c dependency check inside the rollback transaction.
  4. Make the security-invoker reader role's required source privileges explicit and verifiable.

LOW:

  1. Materialize §7.2 into one immutable governed migration artifact rather than an assembly recipe.
  2. Prove source-code uniqueness/multiplicity assumptions used by LIMIT 1 lifecycle probes.

Detailed patch acceptance criteria are in reports/codex-pidx-v0.3.1-required-patches-2026-06-23.md.

12. Final recommendation

Patch v0.3.1 then re-review.

Do not proceed to Owner build authorization discussion yet. Produce v0.3.2 with the two HIGH fixes and the safety tests above, keep scope at 2T2V, then perform a focused static re-review of rollback, ACLs, and the canonical idempotency verifier.

INCOMEX evidence (actual output):

$ wc -l -c codex-pidx-v0.3.1-static-review-2026-06-23.md codex-pidx-v0.3.1-required-patches-2026-06-23.md
236 16433 codex-pidx-v0.3.1-static-review-2026-06-23.md
 99  5330 codex-pidx-v0.3.1-required-patches-2026-06-23.md
335 21763 total

$ rg '^## ([1-9]|1[0-2])\.' main-report
## 1 through ## 12: 12 headings present in order

$ rg executable relkind concatenations
714: c.relkind::text
925: c.relkind::text
1008: c.relkind::text
uncast executable matches: 0

Agent Data upload/read-back:
main report: updated revision=2; read-back length=17313; evidence/final-heading=true
patch report: created revision=1; read-back length=5304; title/HIGH-1=true

Step disposition: OR/Constitution/relevant laws read; no OR update required; the required-patches report is the explicit handoff/technical-debt record. Production verification is intentionally N/A because the mission forbids build and runtime mutation.

Back to Knowledge Hub knowledge/dev/laws-new/workflow-manage/reports/codex-pidx-v0.3.1-static-review-2026-06-23.md