Incomex AI Portal
KB-2FF7 rev 2

Codex Focused Static Review — PIDX v0.3.2

16 min read Revision 2
workflow-managepidxv0.3.2focused-static-reviewcodex2026-06-23

Codex Focused Static Review — PIDX v0.3.2

1. Executive verdict

PASS_WITH_CAVEATS_REQUIRES_PATCH

Focused answers:

  1. HIGH-1 canonical fingerprint: PARTIAL, not resolved. The facet coverage is materially complete, but the state-classification query calls an undefined pidx_build_assertions(). The proposed helper is neither created nor fingerprinted and would add a runtime object outside the claimed 2T2V surface. The alternative inline form is prose, not executable candidate SQL.
  2. HIGH-2 rollback proof: PARTIAL, not resolved. Seed allowlist/hash and writer provenance restrictions are good. However, the rollback DO block contains psql variables inside a dollar-quoted body, where psql does not interpolate them, so the block is not executable as written. Separately, the manifest can point its “archive” relation names at the live PIDX tables themselves; counts/hashes then pass and DROP destroys the only copy.
  3. MEDIUM-1 identity INSERT: RESOLVED. Column-level INSERT excludes both identity columns, so OVERRIDING SYSTEM VALUE cannot bypass the column privilege check.
  4. New BLOCK/HIGH regression: no architectural regression, but the two executable/safety defects above remain HIGH build-gate failures.
  5. Owner build authorization discussion: not ready.

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

2. Files read

Rules/laws queried directly through Agent Data in the main process:

  • .claude/skills/incomex-rules.md — full 36-rule/8-step skill;
  • knowledge/dev/ssot/operating-rules.md v7.58;
  • knowledge/dev/ssot/vps/vps-operating-rules.md v1.0;
  • knowledge/dev/laws/constitution.md current v4.6.3 (v4.0 lineage);
  • knowledge/dev/laws/law-01-foundation-principles.md v3.3;
  • knowledge/dev/laws/dieu33-postgresql-law.md v2.1;
  • PIDX/workflow-manage foundation context.

Required prior reports:

  • reports/codex-pidx-v0.3.1-static-review-2026-06-23.md;
  • reports/codex-pidx-v0.3.1-required-patches-2026-06-23.md.

Required v0.3.2 package:

  • design/pidx-ddl-candidate-v0.3.2.sql.md — all 646 lines;
  • design/pidx-build-access-idempotency-contract-v0.3.2.md;
  • design/pidx-test-plan-v0.3.2.md;
  • design/pidx-codex-review-packet-v0.3.2.md;
  • reports/pidx-v0.3.2-patch-from-codex-2026-06-23.md;
  • reports/pidx-build-design-go-no-go-v0.3.2.md;
  • the three v0.3.2 architecture/readiness/seed delta stubs.

The v0.3.2 package was not returned by Agent Data semantic search, so it was read from the exact workspace paths specified by the mission. Reports from this review are uploaded/read back through Agent Data during AP-CLOSE.

PostgreSQL behavior was checked against official PostgreSQL 16 documentation for psql SQL interpolation, identity INSERT privileges, and access privilege inquiry functions.

3. HIGH-1 canonical fingerprint review

Coverage

The new facet model is technically strong:

  • exact four known table/view names and relation kinds;
  • schema fixed to public;
  • owners and sorted reloptions;
  • ordered column name/type/not-null/default/identity/generated;
  • PK/UNIQUE/CHECK/FK definitions including FK actions;
  • every index attached to the four relations;
  • view definitions;
  • normalized table and column ACL facets;
  • expected value derived from a reviewed sandbox artifact rather than learned from production.

The attidentity::text and attgenerated::text casts correctly avoid the prior "char" ambiguity.

HIGH-1A — classifier calls a verifier that does not exist

At DDL line 236:

AND (SELECT bool_and(passed) FROM pidx_build_assertions())

No CREATE FUNCTION pidx_build_assertions exists anywhere in the candidate. §7.4 is a standalone CTE query, not a set-returning function. The note at line 243 says a future build may create the helper or inline the battery, but neither final form is supplied.

Consequences:

  • §7.1 cannot execute as written, so it cannot return PRESENT_MATCHING;
  • creating the helper would add an unreviewed runtime function outside 2T2V;
  • the helper itself would not be included in pidx_set or the canonical fingerprint;
  • the promised “one immutable verifier artifact” does not yet exist.

This is HIGH because the mandatory idempotency/no-op gate is non-executable.

HIGH-1B — “exact object set” is narrower than claimed

pidx_set only includes pg_class.relkind IN ('r','v','m','p'). It intentionally excludes indexes/sequences and cannot see a pidx_*() helper function in pg_proc. Attached indexes are fingerprinted, which is good, but an unexpected standalone pidx_* function/index/sequence is not classified as an extra object.

The final design must define the expected auxiliary-object set explicitly (including the two identity sequences) and reject every unexpected PIDX-namespaced object, or strictly inline the assertion battery and prohibit helper objects.

Expected fingerprint

Artifact-derived sandbox capture is acceptable in principle. It becomes reviewable only after the final executable verifier form and exact auxiliary-object policy are fixed and the pinned value is generated from those final bytes.

4. HIGH-2 rollback proof review

Improvements that are valid

  • writer column grants exclude source_ref;
  • seed status uses exact counts, an 18-code allowlist, and two deterministic hashes rather than source_ref alone;
  • fake/missing manifest token and missing relation checks are present;
  • archive counts and hashes are recomputed and compared with live data and manifest metadata;
  • generic pg_depend is rechecked in-transaction;
  • DROP uses RESTRICT semantics and no CASCADE;
  • pidx_archive artifacts are rollback-time governed backups, not runtime readiness truth, so they need not violate 2T2V by themselves.

HIGH-2A — pinned hashes are not available inside the DO block

Lines 533–534 place:

exp_proc_hash text := :'expected_seed_proc_hash';
exp_ingr_hash text := :'expected_seed_ingredient_hash';

inside DO $$ ... $$. A DO body is a dollar-quoted SQL literal. PostgreSQL's psql documentation states that variable interpolation is not performed inside quoted SQL literals. The server therefore receives the colon syntax inside PL/pgSQL, where it is invalid.

The rollback guard cannot execute as written. Pass the values through a server-visible mechanism established outside the quoted block (for example validated transaction-local settings read with current_setting), or generate a final literal-bound reviewed artifact before execution.

HIGH-2B — archive proof can alias the live tables

Guard B checks that m.archive_proc_relname and m.archive_ingr_relname resolve and that their content equals live data. It does not require:

  • namespace = pidx_archive;
  • base-table relkind;
  • archive OIDs distinct from the two live table OIDs;
  • archive relations independent of the live PIDX objects.

A manifest row can therefore name:

public.pidx_procedure
public.pidx_procedure_ingredient

as its archive relations. Existence, counts, hashes, and manifest metadata all match trivially. Guard B proceeds, then the DROP statements destroy those same relations; no recovery copy remains. This directly violates “rollback cannot silently drop real data”.

The guard must resolve archive OIDs once and assert the required namespace, base-table kind, distinct live/archive OIDs, expected column shape, and no dependency on the live PIDX objects before comparing contents.

Other rollback notes

  • Hash projections include source_ref, contradicting the stated “business columns only” rule. This is fail-closed rather than an erasure path, so severity is MEDIUM.
  • Empty-table hashes are NULL, while manifest hash columns are NOT NULL. A partial/empty build cannot produce the stated verified manifest without an explicit empty-set hash. MEDIUM availability/recovery gap.
  • Dynamic relation interpolation uses format('%s', relname); after strict OID/namespace validation, use the resolved regclass value or safely quoted identifiers.

5. MEDIUM-1 identity INSERT review

RESOLVED.

The grants are now column-specific and exclude id for both tables. PostgreSQL requires INSERT privilege on every explicitly targeted column, including when OVERRIDING SYSTEM VALUE is used. The owner/build role remains separate; the seed does not provide fixed IDs.

Positive checks:

  • parent INSERT excludes id;
  • ingredient INSERT excludes id;
  • UPDATE lists exclude both IDs;
  • parent procedure_code UPDATE remains denied;
  • parent DELETE remains denied;
  • A8/A9 explicitly cover identity privilege and behavior.

Non-blocking test gap: A8 does not query UPDATE(id) on the ingredient table, though the actual grant excludes it and the ACL fingerprint would detect drift. Add the symmetric assertion.

6. Regression review

Readiness SQL is inherited unchanged. No lifecycle, usability, UNKNOWN_SOURCE, READ_BLOCKED, warning, or false-READY branch was weakened. The prior 18-row totals remain READY=4 / RWW=2 / NOT_READY=10 / UNMAPPED=2.

No engine, scheduler, event bus, RAG/vector, pg_trgm, Nuxt logic, auto-execution, auto-fix, auto-approval, or Directus runtime dependency was added.

Regression/test findings:

  • MEDIUM: R7 uses DELETE ... LIMIT 1, which is not PostgreSQL syntax; the negative test is not executable as written.
  • MEDIUM: the backup-alias attack above is absent from R4–R8.
  • LOW: R2b gives examples rather than one exact dependency-producing statement with a proven pg_depend row.
  • LOW: S11 checks duplicate data but does not implement its stated check that backing UNIQUE constraints exist.
  • LOW: reader-source assertions rely on existing public schema visibility; add an explicit schema-USAGE assertion for the mapped runtime role.

No new false-READY regression was found.

7. Scope / no-new-truth review

Runtime readiness remains 2T2V:

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

pidx_archive relations can be treated as Owner-gated backup artifacts rather than PIDX runtime truth, provided they are validated as independent recovery copies and never read by readiness views.

The optional pidx_build_assertions() helper is not acceptable under the current 2T2V claim. Inline the battery into the single verifier query, or explicitly expand/review/fingerprint the auxiliary runtime object set.

3 câu Tuyên ngôn

  1. Vĩnh viễn: canonical catalog facets and verified independent backups address root safety risks, but only after the final executable verifier/guard replaces the current prose alternatives.
  2. Nhầm được không: currently yes—an operator can supply live tables as “archive” names, and the classifier references an undefined helper. Infrastructure does not yet make the mistake impossible.
  3. 100% tự động: not yet—the final verifier form and pinned-value injection method still require manual interpretation/assembly. The patch must produce one immutable executable verifier and rollback guard.

8. Owner-decision items before build

  1. Điều 33 classification: PG-native technical index or governed Directus entity.
  2. PG-native-only access versus Directus registration/permissions.
  3. Registered migration DOT + paired verifier and APR authorization route.
  4. Final role mapping for owner/writer/reader and context_pack_readonly membership.
  5. Inline assertion battery versus an explicitly governed/fingerprinted auxiliary verifier object; recommendation: inline to preserve 2T2V.
  6. Exact expected auxiliary-object policy, including identity sequences and prohibited PIDX-namespaced extras.
  7. How pinned canonical/seed hashes are bound into the immutable artifact without psql interpolation inside quoted blocks.
  8. Backup schema/owner/retention and independent-archive validation policy.
  9. Whether 18 fixtures persist or remain controlled build tests.
  10. Acceptance of io/checker/template/report = UNKNOWN_SOURCE.

These are decisions only; no authorization is requested.

9. Required patches

BLOCK: none.

HIGH:

  1. Replace the undefined pidx_build_assertions() call with one complete inline verifier query, or explicitly add/govern/fingerprint the helper and update the scope. Define and enforce the exact PIDX auxiliary-object set.
  2. Make rollback Guard B executable by binding expected seed hashes outside the dollar-quoted block through a valid server-visible mechanism.
  3. Require archive relations to be independent base tables in the approved backup namespace, distinct from live PIDX OIDs and not dependent on live PIDX objects.

MEDIUM:

  1. Remove non-business provenance from the seed content projection or document/pin it as immutable artifact metadata.
  2. Define a deterministic empty-set hash for zero-row/partial-build recovery.
  3. Replace R7's invalid DELETE ... LIMIT 1 with valid PostgreSQL corruption SQL.
  4. Add the archive-alias negative test and the missing symmetric ingredient UPDATE(id) assertion.

LOW:

  1. Make R2b and S11 executable/provable rather than descriptive.
  2. Assert schema USAGE and mapped-role membership explicitly.

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

10. Final recommendation

Patch v0.3.2 then re-review.

Produce v0.3.3 with a single executable inline canonical verifier, a valid pinned-hash binding mechanism, and archive-independence enforcement plus negative tests. Then perform one focused static re-review. Do not proceed to Owner build authorization discussion yet.

INCOMEX step disposition:

  • Step 0–2: mission, skill, OR, Constitution/laws, prior findings, and all v0.3.2 artifacts read.
  • Step 3–4: Coder hat confirmed the intended facets/grants; Reviewer hat attacked executable composition, psql quoting, and archive trust boundaries.
  • Step 5: production verification is intentionally N/A because build/DDL/DML are prohibited.
  • Step 6: report + required-patches are the handoff/TD record; no OR update is required because no new operating principle was introduced.

Actual static evidence:

$ wc -l -c main-report required-patches
248 14418 codex-pidx-v0.3.2-focused-static-review-2026-06-23.md
 79  4354 codex-pidx-v0.3.2-required-patches-2026-06-23.md
327 18772 total

$ rg required report headings
## 1 through ## 10: 10 headings present in order

$ rg critical candidate sites
236: FROM pidx_build_assertions()
533: exp_proc_hash := :'expected_seed_proc_hash'  [inside DO $$]
534: exp_ingr_hash := :'expected_seed_ingredient_hash' [inside DO $$]
575: dynamic archive query from manifest relation text
112(test plan): DELETE ... LIMIT 1

Agent Data first upload/read-back:
main: created revision=1, length=14932, verdict/final-heading=true
patches: created revision=1, length=4324, HIGH-3=true
Back to Knowledge Hub knowledge/dev/laws-new/workflow-manage/reports/codex-pidx-v0.3.2-focused-static-review-2026-06-23.md