Codex Static Review — PIDX v0.3.1
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:
- the idempotency fingerprint can classify materially drifted constraints, indexes, view security, ownership, and grants as
PRESENT_MATCHING; - rollback trusts writer-controlled
source_refvalues 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.mdv7.58 rev 51;knowledge/dev/ssot/vps/vps-operating-rules.mdv1.0 rev 2;knowledge/dev/laws/constitution.mdcurrent v4.6.3 rev 44 (the requested v4.0 lineage);knowledge/dev/laws/law-01-foundation-principles.mdv3.3;knowledge/dev/laws/dieu20-thiet-ke-truoc-trien-khai.md;knowledge/dev/laws/dieu33-postgresql-law.mdv2.1;knowledge/dev/laws/law-08-dependency.md;knowledge/dev/laws-new/workflow-manage/de-bai-quan-ly-quy-trinh.mdrev 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, includingsecurity_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 VALUEcontradicts 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 ... RESTRICTis 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_readerreceives the four PIDX SELECT grants but not the source-registry grants required by security-invoker views. Existing direct privileges ofcontext_pack_readonlymay 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_writerhas 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_refpasses. 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:
- mutate each omitted fingerprint dimension (CHECK/FK/index/
security_invoker/grant/owner/identity mode) and requirePARTIAL_OR_DRIFTED; - attempt explicit parent and ingredient IDs with
OVERRIDING SYSTEM VALUEaspidx_writerand require denial; - create a non-seed row, retag it with the seed literal, and prove rollback still aborts;
- set a fake non-empty backup GUC with no archive and prove rollback aborts;
- corrupt archive row count/checksum and prove rollback aborts;
- add a non-rewrite, non-FK normal dependency and prove the in-transaction generic dependency guard catches it;
- execute view/source readability assertions under the actual mapped reader role.
10. Owner-decision items before build
- Điều 33 classification: PG-native technical index or governed Directus entity.
- PG-native prototype access versus Directus registration/permissions.
- Governed build route: registered migration DOT + paired verifier, authorized by APR, or another approved route.
- Role provisioning/mapping for owner, writer, reader, and
context_pack_readonly. - Canonical expected-definition strategy: artifact-derived hash plus complete catalog/ACL assertions.
- Backup destination and machine-verifiable archive manifest policy.
- Whether the 13 base / 18 total fixtures persist or are build-test-only.
- Acceptance rule for
io/checker/template/report = UNKNOWN_SOURCE. - Whether PIDX's surrogate IDs must be system-generated only; recommendation: yes.
- 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:
- 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. - 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:
- Use column-level INSERT grants excluding
idand add an explicitOVERRIDING SYSTEM VALUEdenial test. - Remove writer mutation of system provenance, or remove provenance from rollback authorization.
- Re-run the generic P-2c dependency check inside the rollback transaction.
- Make the security-invoker reader role's required source privileges explicit and verifiable.
LOW:
- Materialize §7.2 into one immutable governed migration artifact rather than an assembly recipe.
- Prove source-code uniqueness/multiplicity assumptions used by
LIMIT 1lifecycle 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.