Skip to content

feat(persistence): enforce migration object naming contracts - #521

Draft
seonghobae wants to merge 314 commits into
fix/contradictory-zero-count-recordsfrom
feat/migration-naming-object-coverage
Draft

seonghobae wants to merge 314 commits into
fix/contradictory-zero-count-recordsfrom
feat/migration-naming-object-coverage

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Gap / owner boundary

AGENTS contract 12 requires durable database object names to contain at least two words and use snake_case by default. This Draft also owns bounded PostgreSQL final-state/execution-context validation for RLS, role, trigger, routine, table, retention, session-execution and persistent login-default state. It remains a bounded validator, not a complete PostgreSQL/procedural-language parser.

Temporal/event/longitudinal scientific truth remains TEPP-owned. Reusable static psychometric arithmetic remains fast-mlsirm-owned; semantic LLM routing remains contextual-orchestrator-owned. No foreign-owner source copy or mutable-PR dependency is authorized.

Current stack authority

  • current exact head: 0fa4b71c0e8cd88d95c70c806af648e853ea7b08;
  • base/foundation successor: fix(coverage): reconcile contradictory LCOV opener counts #538 exact f335624cea977fea7d7fe6f36871868557fe2bb7;
  • exact compare: ahead 314 / behind 0, 69 changed files;
  • state: open / Draft / mergeable;
  • ordinary-forward foundation inheritance commit: 0fa4b71c..., with parents 2e3c57bc... and f335624....

When #538 advanced by three ordinary-forward commits for #594, this dependent PR became diverged (ahead 313 / behind 3) and temporarily non-mergeable. The repair did not rebase or force-push. The merge commit preserves the full #521 history and inherits only the two #538 file deltas absent from the old base: .github/workflows/hourly-nim-product-development.yml and tests/quality/test_hourly_nim_cache_boundary.py. Exact comparison now proves #538 is the merge base and this branch is behind by 0.

Foundation lifecycle/security/CodeQL prerequisites are inherited, not copied or bypassed here. #538 is Ready/open/mergeable at f335624... but its new exact-head required workflows are still incomplete. #498 Dependency Graph support, canonical .github#2276 GHAS configuration-identity read authority, exact-head proof that #594 removed the two Actions cache-poisoning findings, and qualifying independent approval remain prerequisites. No merge or predecessor-evidence transfer is authorized while those gates remain incomplete.

Current bounded persistence contract

crate::migration is the public facade. Forward and rollback SQL cross one PostgreSQL-aware lexical authority before structural validation; #560 owns committed-statement projection so rolled-back statements cannot donate final-state evidence.

#564#590 progressively bind final RLS/policy/table/trigger/append-only/retention routine state plus session_replication_role through direct SET, canonical set_config, writable pg_settings.setting, CTE/data-modifying CTE, row assignment, opaque immediate DO/CALL, Unicode-escaped identifiers and persistent role/user/database/system defaults. The architectural endpoint remains first-class final table/policy/trigger/routine/retention and execution/default-context state, canonical relation identity, plus one shared PostgreSQL Unicode identifier decoder. RESET/DEFAULT/FROM CURRENT precedence is still conservatively fail-closed pending that aggregate.

#590 remains the last accepted production repair in this lane: schema qualification with whitespace around . could alias sibling table RLS state inside the bounded projector; 2279ede859f304b70d43aded832a7dfca84b0de6 fails closed on those unsupported qualified identities. Scoped review found no additional #590 source-level bypass or false positive, but that is not hosted GREEN or qualifying approval.

#591 hypothesis retirement

A subsequent hypothesis proposed rejecting ALTER TABLE ... SET SCHEMA as an unsafe final-state mutation. Review showed the current TEPP contract has no schema-bound owner/ACL/search_path invariant, while PostgreSQL schema movement preserves the relation object and its attached columns/RLS/policy/trigger dependencies. Treating schema movement alone as unsafe would therefore add an unsupported false positive.

Ordinary-forward history is preserved:

  • b741bca4d72b35857b78645d07d81f96b96ab451 — public RED for the hypothesis;
  • 2e3c57bcabff660a69c2ce5fcae45ffd1424bbd9 — retires that RED without any production repair or history rewrite;
  • 0fa4b71c0e8cd88d95c70c806af648e853ea7b08 — preserves that retirement while inheriting the new foundation head ordinary-forward;
  • issue fix(persistence): reject ALTER TABLE SET SCHEMA until relation identity is modeled #591 is closed not planned / no valid delta. Reopen only if TEPP later introduces a concrete schema-bound invariant.

This retirement is intentional scientific/contract hygiene: fail-closed behavior is required only when a valid owned invariant is actually threatened.

Documentation / topology single-writer handoff

Historical pre-#543 53/53 rustdoc evidence is not current acceptance. #589/#590 changed semantic/rustdoc topology; #591 adds no production denominator after retirement. #539 must remeasure the whole owned-production topology only after #538 lands and #521 is subsequently restacked from protected main to a new exact head.

docs/product-technical-gap-baseline.md and docs/TRACEABILITY.md remain in #435's single-writer lane; accepted PostgreSQL authority, the rejected #591 hypothesis, and foundation security prerequisites belong there as decision evidence rather than parallel edits from this branch.

Promotion boundary

The current merge from #538 only keeps the dependent stack coherent while the foundation is still open; it is not the post-landing restack. After #538 lands normally, surviving #521 delta must still be non-force restacked onto protected main; that new exact head must reacquire Rust/Python/docs/security/Semgrep/CodeQL, Live PostgreSQL, owned-production rustdoc 100%, authored line/branch/edge 100%, resolved valid findings, applicable scientific/recovery evidence, and qualifying independent review before Ready promotion.

No blind rerun, self-approval, bypass, force push, destructive rebase, skip/xfail, threshold/denominator manipulation, source rewriting, mutable foreign dependency, or predecessor-head evidence transfer is authorized.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

마이그레이션 검증 구현을 카탈로그·어휘 정규화·구조 검증·RLS 검증 모듈로 분리했다. 정방향과 롤백 SQL을 모두 검사하고, 식별자·테이블 계약·RLS 정책·역할 보안 상태를 더 엄격하게 검증하는 테스트를 추가했다.

Changes

마이그레이션 계약 검증

Layer / File(s) Summary
카탈로그 API와 SQL 어휘 정규화
crates/persistence_postgres/src/migration.rs, crates/persistence_postgres/src/migration_core.rs, crates/persistence_postgres/src/migration_validation.rs, crates/persistence_postgres/tests/migration_*lexical_contract.rs, crates/persistence_postgres/tests/migration_escape_string_contract.rs
MigrationCatalog 생성·접근 API와 임베디드 SQL 결합을 추가했다. 문자열, 주석, 인용 식별자, 달러 인용, ROLE/USER/GROUP 별칭과 역할 생명주기를 정규화하고 검증한다.
구조적 마이그레이션 계약
crates/persistence_postgres/src/migration_core.rs, crates/persistence_postgres/src/naming.rs, crates/persistence_postgres/src/migration.rs, crates/persistence_postgres/tests/migration_concurrent_index_contract.rs, crates/persistence_postgres/tests/migration_create_table_as_contract.rs, crates/persistence_postgres/tests/migration_exact_column_contract.rs, crates/persistence_postgres/tests/migration_identifier_length_contract.rs, crates/persistence_postgres/tests/migration_table_prefix_contract.rs
객체·컬럼·컨스트레인트 이름, 테이블 본문, tenant 및 시간 컬럼, append-only·temporal·retention 계약을 검증한다. PostgreSQL 63바이트 식별자 제한과 CREATE INDEX CONCURRENTLY 구문을 처리한다.
RLS 테넌트 격리 계약
crates/persistence_postgres/src/migration.rs, crates/persistence_postgres/src/migration_core.rs, crates/persistence_postgres/tests/migration_rls_*contract.rs
정확한 current_setting 호출, tenant 식별자 바인딩, Boolean 경로, USING/WITH CHECK, 명령·역할 범위와 permissive/restrictive 정책 대응을 검증한다. 런타임 역할의 SUPERUSERBYPASSRLS 상태와 역할 변경·삭제도 검사한다.

Priority: ⬇️ Low

Estimated code review effort: 5 (Critical) | ~90 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant MigrationCatalog
  participant SQLNormalizer
  participant ContractValidator
  participant RLSValidator
  MigrationCatalog->>SQLNormalizer: 정방향·롤백 SQL 전달
  SQLNormalizer->>ContractValidator: 정규화된 SQL 전달
  ContractValidator->>RLSValidator: RLS 선언과 정책 전달
  RLSValidator-->>ContractValidator: RLS 계약 결과
  ContractValidator-->>MigrationCatalog: 검증 결과 반환
Loading

Merge Risk: 🔵 Low · up to 5474d

Valid migrations containing dollar signs in unquoted identifiers can be rejected before application. The localized lexer fix should be made before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 187 functions across 19 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 마이그레이션 객체 명명 계약 강화를 정확히 나타냅니다. 변경 사항에는 RLS, 역할 수명 주기, 어휘 분석 등 추가 계약도 포함되지만 제목은 주요 변경 범위와 관련됩니다.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/migration-naming-object-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as draft September 14, 2026 19:09
@seonghobae

Copy link
Copy Markdown
Contributor Author

Both failures on this head are now addressed

The first verdict on this branch reported three failing checks. Each had a different cause and each is handled.

Repository contracts and Python branch coverage was inherited. This branch descends from #520, which descends from main, and main still carries the stale cron: "47 * * * *" assertion. I merged #517's branch into #520 and then #520 into this one, without a force push, so both now carry the repair as an ancestor.

Format, lint, test, rustdoc, and dependency policy is RUSTSEC-2026-0285 in the pinned rustls. It fails on every branch in this repository regardless of content and is repaired in #526.

Production line and branch coverage was a real gap in this branch, and I had missed it. My earlier local verification measured line coverage for migration.rs and naming.rs, and measured workspace branch coverage on a different branch. Neither covered this one.

Reproducing with the pinned nightly that CI uses:

cargo +nightly-2026-08-21 llvm-cov --branch --workspace --all-features --json \
  --ignore-filename-regex 'sqlx_live\.rs|mlx_native_receipt'

Two arms had never been reached, both introduced by this change:

Site Counts
is_word_start boundary test true 513, false 0
parse_column_names emptiness guard true 528, false 0

is_word_start had never seen a keyword preceded by an alphanumeric character. Every fixture preceded the keyword with whitespace or an underscore, so the first operand of its boundary test never evaluated false. parse_column_names had never seen a table-body segment whose leading identifier is empty, so the emptiness half of its guard never evaluated false.

Two fixture lines reach both: a comment containing 1CONSTRAINT, and an empty top-level segment inside a CREATE TABLE body. Neither declares an object, so every assertion is unchanged.

After the change migration.rs has no uncovered arm. The only remaining uncovered arms in persistence_postgres are in sqlx_live.rs, which the CI regex excludes.

Local evidence at the new head

Gate Result
cargo fmt --all -- --check clean
cargo clippy --workspace --all-targets --all-features -- -D warnings 0 errors, 0 warnings
cargo test --workspace --all-features 314 test binaries ok
cargo test -p persistence_postgres --lib 62 passed, 0 failed
cargo +nightly-2026-08-21 llvm-cov --branch -p persistence_postgres 0 uncovered arms outside sqlx_live.rs
python3 -m unittest discover -s tests/quality OK
git diff --check clean

@seonghobae

Copy link
Copy Markdown
Contributor Author

Re-verified after the restacks, on the current head

This branch has been through several restacks by other sessions since I last ran the Rust gates on it, including a merge of #526's rustls repair. Re-running everything on 326b3e86:

Gate Result
cargo fmt --all -- --check clean
cargo clippy --workspace --all-targets --all-features -- -D warnings 0 errors, 0 warnings
cargo test --workspace --all-features 314 test binaries ok
cargo test -p persistence_postgres --lib 62 passed, 0 failed
cargo +nightly-2026-08-21 llvm-cov --branch --workspace PASS 3898/3898, 100%

The 62 in persistence_postgres includes the naming-contract tests this PR adds, so the restacks did not drop them.

The branch-coverage denominator is worth stating: 3898, against 3884 on plain main. The 14 extra arms are this PR's own additions to validate_migration_catalog, and all of them are covered. That is the check I would want a reviewer to make, because a coverage percentage alone cannot distinguish "the new code is tested" from "the new code was not counted".

Nothing needed fixing. Posting the numbers so the verification is on the record at a head that several sessions have touched.

seonghobae added a commit that referenced this pull request Sep 16, 2026
Preserve the migration-object naming delta while inheriting the current #538 foundation tree. This is an ordinary two-parent reconciliation: the existing #521 head remains first parent and #538 exact 0a6947b is the second parent. No force update or destructive rebase is used.
@seonghobae
seonghobae changed the base branch from fix/migration-naming-case-contract to fix/contradictory-zero-count-records September 16, 2026 06:59

Copy link
Copy Markdown
Contributor Author

Current-head review found a still-valid naming-contract bypass after the non-force restack onto #538. leading_identifier() only consumes ASCII alphanumeric/underscore at the start of a token. A quoted identifier therefore produces an empty name and is silently omitted by both parse_names_after() and parse_column_names(). On a conforming unquoted table, a declaration such as CREATE INDEX "Bad" ON tenant_record (tenant_record_id); can therefore evade AGENTS contract 12 instead of returning SingleWordObjectName.

This PR must remain Draft. Repair acceptance: add a public validate_migration_catalog() RED covering at least a quoted created object and quoted column, parse PostgreSQL quoted identifiers without allocating/normalizing away their declared spelling (including doubled-quote escaping or fail closed on malformed quoting), then prove valid unquoted/lower-snake migrations remain unchanged. Do not weaken the naming predicate or treat an empty parse as success. The current restack commit 6f5e3b7da0038c0b0d187beb8972cddb1e2fa6b6 is stack repair evidence only, not promotion evidence.

Copy link
Copy Markdown
Contributor Author

A second current-head review finding belongs in the same parser repair rather than a separate workaround. parse_names_after() searches sql.to_ascii_uppercase() directly and only checks identifier boundaries around the keyword. It does not track SQL single-quoted literals, -- line comments, or /* ... */ block comments. The existing regression proves that integrity_constraint_violation and CREATE TABLEX are not matches, but it does not cover a complete declaration-shaped token sequence inside trivia. For example, -- CREATE INDEX Bad ON tenant_record (tenant_record_id); or SELECT 'CREATE INDEX Bad ON tenant_record'; can be scanned as a real declaration and incorrectly fail the migration naming contract.

Please treat this and the quoted-identifier omission as one lexical-boundary defect. The RED batch should cover comment/string false positives plus quoted object/column false negatives; the causal fix should scan SQL lexical state once (code vs single-quoted literal vs double-quoted identifier vs line/block comment), preserve declared identifier spelling, and fail closed on malformed quoting/comment state. Do not add more raw substring exceptions.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 5e745976c40c824f4fa2b8a321064dc3a3baee75, especially the canonical crate::migration facade/core split, PostgreSQL lexical normalization boundary, quoted-identifier handling, dollar-quoted/comment masking, malformed-region fail-closed behavior, and the adjacent atomic-literal regression. Treat predecessor evidence as non-authoritative; this remains Draft and skipped Draft CI is not GREEN.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Current head moved ordinary-forward to 6b1afbce8fdb90c7a7e6c4a613d4ef2dcbbe8bfe after a public RED (9b47019...) proved doubled-quote identifier prefix truncation. Please review this exact head rather than 5e745976.... Focus on whether the lexical facade can still synthesize structural SQL or truncate a quoted identifier into a conforming prefix. Draft/skipped CI remains non-promotion evidence.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== HEAD =='
git rev-parse HEAD
git status --short
printf '%s\n' '== migration files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$|migration'
printf '%s\n' '== changed paths from first parent =='
git diff --name-status HEAD^ HEAD
printf '%s\n' '== Rust outlines =='
for f in $(fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$'); do
  echo "--- $f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done
printf '%s\n' '== relevant symbols and tests =='
rg -n -C 5 -P '\b(validate_migration_catalog|normalize|parse_names_after|parse_column_names|leading_identifier|dollar|EmptyMigrationSql|is_word_start)\b' --glob '*.rs'
printf '%s\n' '== facade/core diff =='
git diff --find-renames --find-copies HEAD^ HEAD -- '**/migration.rs' '**/migration_core.rs' '**/*migration*.rs'

Length of output: 997


@seonghobae, repository access failed during the clone step. I could not inspect commit 5e745976c40c824f4fa2b8a321064dc3a3baee75.

The review is inconclusive. Please retry the review after repository access is available.

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== checked-out commit =='
git rev-parse HEAD
git status --short

printf '%s\n' '== migration-related Rust files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$'

printf '%s\n' '== current-head changed paths =='
git diff-tree --no-commit-id --name-status -r HEAD

printf '%s\n' '== symbol outlines =='
while IFS= read -r file; do
  printf '\n--- %s ---\n' "$file"
  ast-grep outline "$file" --items all --type function,struct,enum,impl 2>/dev/null || true
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$')

printf '%s\n' '== validation, lexical, parser, and test references =='
rg -n -C 4 -P '\b(validate_migration_catalog|normalize_migration|migration_core|parse_names_after|parse_column_names|leading_identifier|dollar|EmptyMigrationSql|SingleWordObjectName|is_word_start)\b' --glob '*.rs' .

printf '%s\n' '== facade/core changes against first parent =='
git diff --find-renames --find-copies HEAD^ HEAD -- '**/migration.rs' '**/migration_core.rs' '**/*migration*.rs'

printf '%s\n' '== complete current migration module sources =='
while IFS= read -r file; do
  printf '\n--- %s ---\n' "$file"
  cat -n "$file"
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core)\.rs$')

Length of output: 1378


@seonghobae, repository access failed during the clone step again.

I could not inspect exact head 6b1afbce8fdb90c7a7e6c4a613d4ef2dcbbe8bfe. The review is inconclusive. Please retry the review when repository access is available.

You are interacting with an AI system.

@seonghobae seonghobae changed the title feat(persistence): cover every declared object in the naming contract feat(persistence): enforce migration object naming contracts Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 2526233ccd3a915d7f2662535cbf4c39a67ea335, including the materialized-view RED 77fbd5dc6031f2f2a52ee15ea4273ef2f7d471dd and causal lexical canonicalization. Treat predecessor evidence as diagnostic only; this Draft is not GREEN and #538 remains the foundation prerequisite.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== checked-out revision =='
git rev-parse HEAD
git status --short

echo '== relevant Rust files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$'

echo '== paths changed since foundation base =='
git diff --name-status 0a6947b11f2dba9e161f2abe8bdec0f652d4b47a HEAD -- . || true

echo '== migration module outlines =='
while IFS= read -r f; do
  echo "--- $f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$')

echo '== relevant definitions, callers, and tests =='
rg -n -C 5 -P '\b(validate_migration_catalog|normalize_migration_sql|parse_names_after|parse_column_names|leading_identifier|CREATE MATERIALIZED VIEW|CREATE VIEW|dollar|EmptyMigrationSql|SingleWordObjectName|is_word_start)\b' --glob '*.rs' .

echo '== change diff for migration surface =='
git diff --find-renames 0a6947b11f2dba9e161f2abe8bdec0f652d4b47a HEAD -- '**/migration.rs' '**/migration_core.rs' '**/live_repository.rs' '**/*migration*.rs' || true

Length of output: 1194


@seonghobae, repository access failed during the clone step.

I could not inspect exact head 2526233ccd3a915d7f2662535cbf4c39a67ea335, the materialized-view RED, or the lexical canonicalization. The review is inconclusive.

Please retry the review when repository access is available.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 3f272890da4dbc527de247eff6faa8427171e4ef, focusing on the public RED cb856ec570fc9c2a68562b1212fc2c3915ccae32 and the structural canonicalization of CREATE OR REPLACE VIEW onto the existing CREATE VIEW naming authority. Treat this Draft's skipped CI as non-evidence and predecessor checks as diagnostic only.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== revision =='
git rev-parse HEAD
git status --short
printf '%s\n' '== relevant Rust files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$'
printf '%s\n' '== delta paths from foundation =='
git diff --name-status 0a6947b11f2dba9e161f2abe8bdec0f652d4b47a HEAD -- .
printf '%s\n' '== outlines =='
while IFS= read -r f; do
  printf '\n--- %s ---\n' "$f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core|live_repository)\.rs$')
printf '%s\n' '== targeted source references =='
rg -n -C 6 -P '\b(validate_migration_catalog|normalize_migration_sql|migration_core|parse_names_after|parse_column_names|leading_identifier|is_word_start|EmptyMigrationSql|SingleWordObjectName)\b|CREATE (OR REPLACE|MATERIALIZED )?VIEW|dollar' --glob '*.rs' .
printf '%s\n' '== migration delta =='
git diff --find-renames 0a6947b11f2dba9e161f2abe8bdec0f652d4b47a HEAD -- '**/migration.rs' '**/migration_core.rs' '**/live_repository.rs' '**/*migration*.rs'
printf '%s\n' '== requested RED commit =='
git show --format=fuller --find-renames --stat cb856ec570fc9c2a68562b1212fc2c3915ccae32
git show --format= --find-renames cb856ec570fc9c2a68562b1212fc2c3915ccae32 -- '**/migration.rs' '**/migration_core.rs' '**/*migration*.rs'

Length of output: 2296


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
do
  printf '\n== outline: %s ==\n' "$f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done

printf '%s\n' '== migration facade =='
cat -n crates/persistence_postgres/src/migration.rs
printf '%s\n' '== migration core =='
cat -n crates/persistence_postgres/src/migration_core.rs
printf '%s\n' '== migration validation =='
cat -n crates/persistence_postgres/src/migration_validation.rs
printf '%s\n' '== lexical contract tests =='
cat -n crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== current head diff from foundation for migration surface =='
git diff --find-renames --find-copies 0a6947b11f2dba9e161f2abe8bdec0f652d4b47a HEAD -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== RED cb856ec patch =='
git show --format= --find-renames cb856ec570fc9c2a68562b1212fc2c3915ccae32 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== callers and exports =='
rg -n -C 4 -P '\bvalidate_migration_catalog\s*\(|\bmod\s+migration(?:_core|_validation)?\b|\bpub\s+use\b.*migration' crates/persistence_postgres --glob '*.rs'

Length of output: 188209


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== requested head metadata and parent =='
git show --no-patch --format=fuller HEAD
git rev-parse HEAD^
printf '%s\n' '== causal head patch =='
git diff --find-renames HEAD^ HEAD -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
printf '%s\n' '== view declarations in checked-in migrations and tests =='
rg -n -C 3 -i -P '\bCREATE\s+(?:MATERIALIZED\s+|OR\s+REPLACE\s+)?VIEW\b' \
  crates/persistence_postgres migrations --glob '*.rs' --glob '*.sql'
printf '%s\n' '== normalization and parser call boundaries =='
rg -n -C 3 -P '\b(canonicalize_structural_keywords|normalize_migration_sql|parse_created_object_names|parse_names_after)\s*\(' \
  crates/persistence_postgres/src --glob '*.rs'

Length of output: 22953


@seonghobae, I reviewed exact head 3f272890da4dbc527de247eff6faa8427171e4ef.

I found no issue in the requested scope.

crate::migration::validate_migration_catalog() normalizes the forward SQL before it calls the core validator. The causal change maps structural CREATE OR REPLACE VIEW tokens to CREATE VIEW. The core then uses the existing CREATE VIEW object parser and is_multi_word_snake_case predicate.

The public RED is present. replaceable_view_names_are_covered_by_the_object_naming_contract rejects CREATE OR REPLACE VIEW Bad with SingleWordObjectName. The lexical unit test also verifies that ordinary, materialized, and replaceable views converge to the same structural form.

This is a source review only. I do not treat skipped Draft CI or predecessor evidence as promotion evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 2d330a2e69bd87f403e3a5c2ebddc5c2d38df1c3. Focus on public RED 5de7ad0974846c42183d22684e79bbf0fdc240ea, fail-closed qualified-name repair e0d3c0def34d264321cb501bc5d2ab39ecffff08, and the ordinary-forward test import correction at the current head. Verify that schema-qualified CREATE names cannot be accepted by prefix truncation and that unqualified existing declarations are unchanged. Draft-skipped CI is non-evidence.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='2d330a2e69bd87f403e3a5c2ebddc5c2d38df1c3'

printf '%s\n' '== exact head =='
git rev-parse HEAD
git status --short
git merge-base --is-ancestor "$expected" HEAD && echo 'expected head is an ancestor of checkout'
git merge-base --is-ancestor HEAD "$expected" && echo 'checkout is an ancestor of expected head'

printf '%s\n' '== requested commit metadata =='
git show --no-patch --format='%H%n%P%n%s' "$expected"
git show --no-patch --format='%H%n%P%n%s' 5de7ad0974846c42183d22684e79bbf0fdc240ea
git show --no-patch --format='%H%n%P%n%s' e0d3c0def34d264321cb501bc5d2ab39ecffff08

printf '%s\n' '== relevant files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*contract.*\.rs$'

printf '%s\n' '== source outlines =='
for f in crates/persistence_postgres/src/migration.rs crates/persistence_postgres/src/migration_core.rs; do
  if [ -f "$f" ]; then
    echo "--- $f"
    ast-grep outline "$f" --items all --type function,struct,enum,impl
  fi
done

printf '%s\n' '== qualification and parsing implementation =='
rg -n -C 8 -P '\b(qualified|qualification|INVALID|sentinel|canonicalize_structural_keywords|normalize_migration_sql|parse_names_after|parse_column_names|leading_identifier|validate_migration_catalog)\b' \
  crates/persistence_postgres --glob '*.rs'

printf '%s\n' '== tests for qualified and unqualified declarations =='
rg -n -C 6 -i -P '(audit_schema|qualified|unqualified|CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|VIEW)|CREATE\s+(?:UNIQUE\s+)?INDEX)' \
  crates/persistence_postgres --glob '*.rs'

printf '%s\n' '== requested RED patch =='
git show --format= --find-renames 5de7ad0974846c42183d22684e79bbf0fdc240ea -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests \
  crates/persistence_postgres/src

printf '%s\n' '== causal repair and current correction patches =='
git show --format= --find-renames e0d3c0def34d264321cb501bc5d2ab39ecffff08 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests \
  crates/persistence_postgres/src
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests \
  crates/persistence_postgres/src

printf '%s\n' '== checked-in migration CREATE declarations =='
rg -n -i -P '\bCREATE\s+(?:OR\s+REPLACE\s+|MATERIALIZED\s+|UNIQUE\s+|UNIQUE\s+INDEX\s+IF\s+NOT\s+EXISTS\s+)?(?:TABLE|INDEX|VIEW|FUNCTION|TRIGGER|TYPE|SEQUENCE|POLICY|CONSTRAINT)\b' \
  --glob '*.sql' --glob '*.rs' crates/persistence_postgres migrations 2>/dev/null || true

Length of output: 4237


🏁 Script executed:

#!/bin/bash
set -euo pipefail

files=(
  crates/persistence_postgres/src/migration.rs
  crates/persistence_postgres/src/migration_core.rs
  crates/persistence_postgres/src/migration_validation.rs
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
)

printf '%s\n' '== outlines =='
for f in "${files[@]}"; do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    ast-grep outline "$f" --items all --type function,struct,enum 2>/dev/null || true
  fi
done

printf '%s\n' '== relevant implementation and test blocks =='
rg -n -C 12 -P '\b(validate_migration_catalog|normalize_migration_sql|canonicalize_structural_keywords|parse_names_after|parse_column_names|leading_identifier|is_word_start|QUALIFIED|qualified|sentinel|SingleWordObjectName|EmptyMigrationSql)\b' \
  "${files[@]}"

printf '%s\n' '== lexical contract test file =='
if [ -f crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs ]; then
  cat -n crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
fi

printf '%s\n' '== exact causal repair patch =='
git show --format= --find-renames e0d3c0def34d264321cb501bc5d2ab39ecffff08 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== public RED patch =='
git show --format= --find-renames 5de7ad0974846c42183d22684e79bbf0fdc240ea -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== current-head correction patch =='
git show --format= --find-renames 2d330a2e69bd87f403e3a5c2ebddc5c2d38df1c3 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== all checked-in unqualified CREATE declarations in migration SQL =='
fd -t f -e sql . crates/persistence_postgres 2>/dev/null | while IFS= read -r f; do
  rg -n -i -P '\bCREATE\s+(?:OR\s+REPLACE\s+|MATERIALIZED\s+|UNIQUE\s+)?(?:TABLE|INDEX|VIEW|FUNCTION|TRIGGER|TYPE|SEQUENCE|POLICY)\b' "$f" || true
done

Length of output: 123255


@seonghobae, I reviewed exact head 2d330a2e69bd87f403e3a5c2ebddc5c2d38df1c3.

I found one remaining qualified-name bypass.

crates/persistence_postgres/src/migration_validation.rs Line 144 checks for a dot at name_index. For this valid PostgreSQL form, name_index still points to IF:

CREATE TABLE IF NOT EXISTS audit_schema.Bad (...);

The guard only skips IF NOT EXISTS after name_index. It therefore does not replace the qualified name with INVALID_QUALIFIED_IDENTIFIER.

migration_core::leading_identifier() then skips IF NOT EXISTS and accepts the conforming prefix audit_schema. The invalid object segment Bad is not checked.

Add a public validate_migration_catalog() regression for qualified CREATE TABLE IF NOT EXISTS declarations. Then advance past the existence clause before qualification detection for table declarations. The same logic should cover each supported CREATE ... IF NOT EXISTS grammar.

The existing tests cover qualified view declarations and the CREATE UNIQUE INDEX IF NOT EXISTS form. They do not cover this table form.

I did not run build or test commands. Draft-skipped CI remains non-evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Current exact head: 0cd03111bfef95aadab49b486c84eb11c0a2b8e7. Please review the PostgreSQL migration lexical/naming boundary and the new runtime-role declaration evidence path specifically; Draft lifecycle skips hosted promotion gates, so this request is review evidence only.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='0cd03111bfef95aadab49b486c84eb11c0a2b8e7'
base='0a6947b11f2dba9e161f2abe8bdec0f652d4b47a'

printf '%s\n' '== exact checkout =='
git rev-parse HEAD
git status --short
git merge-base --is-ancestor "$expected" HEAD && echo 'expected-head-is-ancestor-of-checkout'
git merge-base --is-ancestor HEAD "$expected" && echo 'checkout-is-ancestor-of-expected-head'
git show --no-patch --format='%H%n%P%n%s' "$expected"

printf '%s\n' '== migration-related Rust files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*(contract|lexing).*\.rs$'

printf '%s\n' '== compact source outlines =='
while IFS= read -r f; do
  printf '\n--- %s ---\n' "$f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*(contract|lexing).*\.rs$')

printf '%s\n' '== targeted implementation, callers, and test context =='
rg -n -C 10 -P '\b(validate_migration_catalog|normalize_migration_sql|canonicalize_structural_keywords|parse_created_object_names|parse_names_after|parse_column_names|leading_identifier|is_word_start|is_multi_word_snake_case|CREATE\s+(ROLE|USER|GROUP)|role|dollar|unterminated|SingleWordObjectName|InvalidMigration)\b' \
  crates/persistence_postgres --glob '*.rs'

printf '%s\n' '== checked-in runtime-role declarations =='
rg -n -C 5 -i -P '\bCREATE\s+(ROLE|USER|GROUP)\b|tepp_app_runtime' --glob '*.sql' --glob '*.rs' .

printf '%s\n' '== current delta from foundation on review surface =='
git diff --find-renames --find-copies "$base" "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/live_repository.rs \
  crates/persistence_postgres/tests

printf '%s\n' '== role RED/fix history reachable from exact head =='
git log --oneline --ancestry-path "$base..$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests | tail -n 40

printf '%s\n' '== role-related patches in exact range =='
git log --format='%H %s' --ancestry-path "$base..$expected" --all -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests \
| while IFS=' ' read -r sha subject; do
    if printf '%s\n' "$subject" | rg -qi 'role|user|group|runtime'; then
      printf '\n--- %s %s ---\n' "$sha" "$subject"
      git show --format= --find-renames "$sha" -- \
        crates/persistence_postgres/src/migration.rs \
        crates/persistence_postgres/src/migration_core.rs \
        crates/persistence_postgres/src/migration_validation.rs \
        crates/persistence_postgres/tests
    fi
  done

Length of output: 4236


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='0cd03111bfef95aadab49b486c84eb11c0a2b8e7'
base='0a6947b11f2dba9e161f2abe8bdec0f652d4b47a'
files='crates/persistence_postgres/src/migration.rs
crates/persistence_postgres/src/migration_core.rs
crates/persistence_postgres/src/migration_validation.rs
crates/persistence_postgres/src/live_repository.rs
crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
crates/persistence_postgres/tests/append_only_migration_contract.rs'

printf '%s\n' '== selected source outlines =='
printf '%s\n' "$files" | while IFS= read -r f; do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
  fi
done

printf '%s\n' '== complete selected sources =='
printf '%s\n' "$files" | while IFS= read -r f; do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    cat -n "$f"
  fi
done

printf '%s\n' '== runtime role declarations and migration catalog paths =='
rg -n -C 8 -i -P '\bCREATE\s+(ROLE|USER|GROUP)\b|tepp_app_runtime|migration.*catalog|validate_migration_catalog' \
  --glob '*.sql' --glob '*.rs' .

printf '%s\n' '== exact-head patch =='
git diff --find-renames --find-copies "$expected^" "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/live_repository.rs \
  crates/persistence_postgres/tests \
  crates/persistence_postgres

printf '%s\n' '== current review-surface delta from foundation =='
git diff --stat "$base" "$expected" -- crates/persistence_postgres
git diff --find-renames "$base" "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/live_repository.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs \
  crates/persistence_postgres/tests/append_only_migration_contract.rs

printf '%s\n' '== runtime-role and role-alias commits =='
git log --format='%H %s' "$base..$expected" -- crates/persistence_postgres | rg -i 'role|runtime|user|group' || true

Length of output: 104223


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== current facade and role-validation call flow =='
rg -n -C 18 -P '\b(validate_migration_catalog|normalize_migration_sql|declares_created_role|validate_tenant_rls_contract|MissingAppRuntimeRole)\b' \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== current lexical and structural functions =='
rg -n -C 14 -P '^fn (lexically_normalize_migration_sql|canonicalize_structural_keywords|scan_single_quoted_literal|scan_quoted_identifier|scan_block_comment|dollar_quote_delimiter|scan_dollar_quoted_body|literal_is_atomic|quoted_identifier_is_structurally_safe)|^pub\(super\) fn (normalize_migration_sql|declares_created_role)' \
  crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== current public role and lexical tests =='
rg -n -C 12 -P '^fn (created_role_aliases_are_covered_by_the_object_naming_contract|runtime_role_reference_does_not_substitute_for_role_declaration|quoted_created_object_cannot_bypass_the_naming_contract|declaration_shaped_text_inside_sql_trivia_is_not_an_object)|role_declaration_evidence_uses_the_lexical_boundary|role_creation_aliases_share_the_created_object_name_scanner' \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs \
  crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== current shipped RLS role declaration context =='
fd -t f -e sql . | rg '0002_tenant_row_level_security\.up\.sql$' | while IFS= read -r f; do
  echo "--- $f"
  rg -n -C 16 -i -P '\bCREATE\s+(ROLE|USER|GROUP)\b|tepp_app_runtime|ROW LEVEL SECURITY|CREATE POLICY' "$f"
done

printf '%s\n' '== exact current-head patch =='
git show --format= --find-renames 0cd03111bfef95aadab49b486c84eb11c0a2b8e7 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== predecessor runtime-role repair patch =='
git show --format= --find-renames dffe2de9fde3019dd2d6e8e78c4d7f8e42e0be1b -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== runtime-role RED patch =='
git show --format= --find-renames 7f63a667dae1eec4057c5cf6de7eada8cf385526 -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

Length of output: 125348


@seonghobae, I reviewed exact head 0cd03111bfef95aadab49b486c84eb11c0a2b8e7.

I found no issue in the requested scope.

crate::migration::validate_migration_catalog() now performs two separate checks on the original forward SQL:

  1. declares_created_role() verifies that tepp_app_runtime has a CREATE ROLE, CREATE USER, or CREATE GROUP declaration.
  2. normalize_migration_sql() creates the structural parser view.

The role-evidence scan uses the lexical-normalization pass before it scans tokens. Therefore, role-shaped text in single-quoted literals, line comments, block comments, and dollar-quoted bodies cannot satisfy the runtime-role requirement. Unterminated lexical regions return None, and the facade returns EmptyMigrationSql.

The structural normalization maps CREATE ROLE, CREATE USER, and CREATE GROUP to the existing one-name CREATE TYPE parser shape. The existing created-object scan then applies is_multi_word_snake_case. The public regression rejects single-word role names for all three PostgreSQL spellings.

The new public regression also verifies that a GRANT ... TO tepp_app_runtime reference does not substitute for a role declaration. The facade performs this check after core validation, so the pre-existing reference check does not hide the missing declaration.

The shipped migration at migrations/0002_tenant_row_level_security.up.sql contains:

CREATE ROLE tepp_app_runtime NOINHERIT NOSUPERUSER NOCREATEDB NOCREATEROLE NOBYPASSRLS;

This declaration satisfies both the new evidence check and the naming contract.

This was a source review. I did not treat Draft lifecycle skips or predecessor checks as promotion evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Current exact head: facb48566c59a88fda8d45b2c22358f48d29f0d5. Please include the role-declaration evidence path and the PostgreSQL CREATE USER versus CREATE USER MAPPING disambiguation in this review. Draft lifecycle evidence remains non-promotional.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='facb48566c59a88fda8d45b2c22358f48d29f0d5'
base='0a6947b11f2dba9e161f2abe8bdec0f652d4b47a'
files=(
  crates/persistence_postgres/src/migration.rs
  crates/persistence_postgres/src/migration_core.rs
  crates/persistence_postgres/src/migration_validation.rs
  crates/persistence_postgres/src/live_repository.rs
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
)

printf '%s\n' '== exact checkout and requested revision =='
git rev-parse HEAD
git status --short
git show --no-patch --format='%H%n%P%n%s' "$expected"
git merge-base --is-ancestor HEAD "$expected" && echo 'checkout-is-ancestor-of-requested'
git merge-base --is-ancestor "$expected" HEAD && echo 'requested-is-ancestor-of-checkout'

printf '%s\n' '== changed migration review surface from foundation =='
git diff --name-status "$base" "$expected" -- crates/persistence_postgres

printf '%s\n' '== selected source outlines =='
for f in "${files[@]}"; do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
  fi
done

printf '%s\n' '== role alias, evidence, lexical boundary, and error contexts =='
rg -n -C 15 -P '\b(validate_migration_catalog|normalize_migration_sql|lexically_normalize_migration_sql|declares_created_role|role_alias|CREATE\s+(ROLE|USER|GROUP)|USER\s+MAPPING|MissingAppRuntimeRole|tepp_app_runtime|canonicalize_structural_keywords|parse_names_after|is_multi_word_snake_case)\b' \
  "${files[@]}" 2>/dev/null || true

printf '%s\n' '== exact requested-head patch =='
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== role-mapping tests and callers =='
rg -n -C 12 -i -P '(user mapping|CREATE USER|CREATE ROLE|CREATE GROUP|runtime.role|role.declaration|MissingAppRuntimeRole|validate_migration_catalog)' \
  crates/persistence_postgres --glob '*.rs'

printf '%s\n' '== checked-in role and SQL/MED declarations =='
rg -n -C 8 -i -P '\bCREATE\s+(ROLE|USER|GROUP)\b|\bCREATE\s+USER\s+MAPPING\b|tepp_app_runtime' \
  --glob '*.sql' --glob '*.rs' .

printf '%s\n' '== role-disambiguation causal patch and reachable history =='
git log --format='%H %s' --ancestry-path "$base..$expected" -- crates/persistence_postgres | rg -i 'role|user mapping|mapping|runtime' || true
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

Length of output: 203284


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== exact revision =='
git rev-parse HEAD
git diff --quiet facb48566c59a88fda8d45b2c22358f48d29f0d5 HEAD || true

printf '%s\n' '== migration facade =='
sed -n '1,80p' crates/persistence_postgres/src/migration.rs

printf '%s\n' '== role evidence and alias recognition =='
sed -n '1,160p' crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== lexical scanner termination behavior =='
sed -n '160,360p' crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== role evidence unit regressions =='
sed -n '360,430p' crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== public lexical contract regressions =='
sed -n '1,190p' crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== all aliases in supported create-name parsing =='
rg -n -C 8 -P '(CREATE TYPE|parse_names_after|parse_created_object_names|CREATE USER MAPPING|is_role_creation_alias)' \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/src/migration_validation.rs

Length of output: 42601


@seonghobae, I reviewed exact head facb48566c59a88fda8d45b2c22358f48d29f0d5.

I found no issue in the requested scope.

crate::migration::validate_migration_catalog() obtains tepp_app_runtime declaration evidence from the original forward SQL. declares_created_role() first applies the lexical boundary. Comments, string literals, quoted bodies, and malformed lexical regions therefore cannot provide role-declaration evidence. A malformed region returns None, and the facade returns EmptyMigrationSql.

is_role_creation_alias() is the shared authority for both paths. It recognizes CREATE ROLE, CREATE USER, and CREATE GROUP. It excludes CREATE USER MAPPING when the token after USER is MAPPING.

The exclusion applies before either operation:

  • declares_created_role() cannot treat CREATE USER MAPPING FOR tepp_app_runtime ... as a tepp_app_runtime role declaration.
  • canonicalize_structural_keywords() does not rewrite CREATE USER MAPPING to CREATE TYPE. The structural parser does not apply the role naming scanner to MAPPING.

The regressions verify both properties. create_user_mapping_is_not_a_role_alias accepts a valid SQL/MED user mapping. role_declaration_evidence_uses_the_lexical_boundary returns Some(false) when that statement mentions tepp_app_runtime. The existing role-alias regression still rejects single-word names for CREATE ROLE, CREATE USER, and CREATE GROUP.

This was a source review. I did not run build or test commands. I do not treat Draft lifecycle skips as promotion evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 1fbe7e5324cd43c1f8508400aac22a7c8806368e, especially the ordered runtime-role final-state logic added after RED cd82ba1afca5ff0cc19195a35328c3f6fe3324d8: DROP ROLE/DROP USER/DROP GROUP, IF EXISTS, comma-separated names, recreation ordering, and DROP USER MAPPING disambiguation. This PR remains Draft; review is diagnostic only and is not promotion authority.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='1fbe7e5324cd43c1f8508400aac22a7c8806368e'
red='cd82ba1afca5ff0cc19195a35328c3f6fe3324d8'
base='0a6947b11f2dba9e161f2abe8bdec0f652d4b47a'

printf '%s\n' '== exact revision =='
git rev-parse HEAD
git show --no-patch --format='%H%n%P%n%s' "$expected"
git merge-base --is-ancestor "$expected" HEAD && echo 'expected-is-ancestor-of-checkout'
git merge-base --is-ancestor HEAD "$expected" && echo 'checkout-is-ancestor-of-expected'

printf '%s\n' '== migration-related Rust files =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*(contract|lexing).*\.rs$'

printf '%s\n' '== compact outlines =='
while IFS= read -r f; do
  printf '\n--- %s ---\n' "$f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done < <(fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*(contract|lexing).*\.rs$')

printf '%s\n' '== final-state implementation and caller context =='
rg -n -C 18 -P '\b(validate_migration_catalog|declares_created_role|MissingAppRuntimeRole|is_role_creation_alias|is_role_drop_alias|DROP\s+(ROLE|USER|GROUP)|USER\s+MAPPING|IF\s+EXISTS|tepp_app_runtime)\b' \
  crates/persistence_postgres --glob '*.rs'

printf '%s\n' '== relevant complete source files =='
for f in \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    cat -n "$f"
  fi
done

printf '%s\n' '== requested RED and causal repair patches =='
git show --format= --find-renames "$red" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests

printf '%s\n' '== final delta from foundation for review surface =='
git diff --find-renames "$base" "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests

printf '%s\n' '== role statements in SQL corpus =='
rg -n -C 8 -i -P '\b(CREATE|DROP)\s+(ROLE|USER|GROUP)\b|USER\s+MAPPING|tepp_app_runtime' \
  --glob '*.sql' --glob '*.rs' .

Length of output: 3891


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='1fbe7e5324cd43c1f8508400aac22a7c8806368e'
red='cd82ba1afca5ff0cc19195a35328c3f6fe3324d8'

printf '%s\n' '== exact revision =='
git rev-parse HEAD
git show --no-patch --format='%H%n%P%n%s' "$expected"

printf '%s\n' '== outlines =='
for f in \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
do
  printf '\n--- %s ---\n' "$f"
  ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
done

printf '%s\n' '== facade and final-state implementation =='
sed -n '1,180p' crates/persistence_postgres/src/migration.rs
sed -n '1,460p' crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== public contract regressions =='
sed -n '1,360p' crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== direct references with context =='
rg -n -C 12 -P '\b(declares_created_role|is_role_creation_alias|is_role_drop_alias|scan_role|DROP\s+(ROLE|USER|GROUP)|USER\s+MAPPING|IF\s+EXISTS|tepp_app_runtime|MissingAppRuntimeRole)\b' \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== RED patch =='
git show --format= --find-renames "$red" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== causal repair patch =='
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== corpus role statements =='
rg -n -C 5 -i -P '\b(CREATE|DROP)\s+(ROLE|USER|GROUP)\b|\b(CREATE|DROP)\s+USER\s+MAPPING\b|tepp_app_runtime' \
  --glob '*.sql' . || true

Length of output: 85289


@seonghobae, I reviewed exact head 1fbe7e5324cd43c1f8508400aac22a7c8806368e.

I found one runtime-role final-state bypass.

declares_created_role() tokenizes normalized SQL with split_whitespace() in crates/persistence_postgres/src/migration_validation.rs Line 13. PostgreSQL does not require whitespace after a semicolon. Therefore, this valid forward SQL bypasses the drop transition:

CREATE ROLE tepp_app_runtime;DROP ROLE tepp_app_runtime;

The token after ROLE is tepp_app_runtime;DROP. role_identifier() still extracts tepp_app_runtime, so the create transition sets declared to true. No later token equals DROP, so is_role_drop_alias() does not run. The validator incorrectly retains runtime-role evidence.

Add a public validate_migration_catalog() regression with RLS and adjacent ;DROP ROLE, ;DROP USER, and ;DROP GROUP statements. The expected result is MissingAppRuntimeRole.

Split statement delimiters during lexical normalization or use statement-aware tokenization before ordered role-state tracking. Do not depend on whitespace to separate SQL statements. The same boundary must preserve valid recreation ordering.

The current DROP USER MAPPING exclusion is correct for whitespace-separated tokens. This finding does not change that requirement.

This was a source review only. I did not treat Draft lifecycle results as promotion evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please re-review exact head f5d3c956d6662bab7e64711d5e509917a73d0f8e. The prior exact-head finding was valid: split_whitespace() allowed CREATE ROLE tepp_app_runtime;DROP ROLE tepp_app_runtime; to hide the drop transition. Public RED e8cc37de946f6bb1a10fe680c1c37e938ac9b5e6 covers adjacent ;DROP ROLE/USER/GROUP; causal fix f5d3c956d6662bab7e64711d5e509917a73d0f8e exposes semicolon/comma structural delimiters as role-lifecycle tokens only after the existing lexical masking pass. Please focus on any remaining valid PostgreSQL delimiter/statement-boundary bypasses or false positives. Draft status remains unchanged; this review is diagnostic, not promotion authority.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='f5d3c956d6662bab7e64711d5e509917a73d0f8e'
red='e8cc37de946f6bb1a10fe680c1c37e938ac9b5e6'

printf '%s\n' '== exact revision =='
git rev-parse HEAD
git show --no-patch --format='%H%n%P%n%s' "$expected"
git merge-base --is-ancestor "$expected" HEAD && echo 'expected-is-ancestor-of-checkout'
git merge-base --is-ancestor HEAD "$expected" && echo 'checkout-is-ancestor-of-expected'

printf '%s\n' '== relevant file discovery =='
fd -t f -e rs . | rg '(^|/)(migration|migration_core|migration_validation|live_repository)\.rs$|migration.*(contract|lexing).*\.rs$'

printf '%s\n' '== outlines =='
for f in \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs
do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    ast-grep outline "$f" --items all --type function,struct,enum,impl 2>/dev/null || true
  fi
done

printf '%s\n' '== role lifecycle and lexical implementation =='
rg -n -C 18 -P '\b(validate_migration_catalog|declares_created_role|is_role_creation_alias|is_role_drop_alias|role_identifier|split_whitespace|semicolon|delimiter|DROP\s+(ROLE|USER|GROUP)|USER\s+MAPPING|IF\s+EXISTS|tepp_app_runtime|lexically_normalize_migration_sql|normalize_migration_sql)\b' \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== complete lifecycle implementation region =='
sed -n '1,260p' crates/persistence_postgres/src/migration_validation.rs
sed -n '260,560p' crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== public role lifecycle regressions =='
rg -n -C 20 -P 'adjacent|delimiter|DROP\s+(ROLE|USER|GROUP)|recreate|IF EXISTS|USER MAPPING|runtime.role|MissingAppRuntimeRole' \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs \
  crates/persistence_postgres/src/migration_validation.rs

printf '%s\n' '== public RED patch =='
git show --format= --find-renames "$red" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== causal repair patch =='
git show --format= --find-renames "$expected" -- \
  crates/persistence_postgres/src/migration.rs \
  crates/persistence_postgres/src/migration_validation.rs \
  crates/persistence_postgres/src/migration_core.rs \
  crates/persistence_postgres/tests/migration_identifier_lexing_contract.rs

printf '%s\n' '== checked-in role SQL forms =='
rg -n -C 6 -i -P '\b(CREATE|DROP)\s+(ROLE|USER|GROUP)\b|\b(CREATE|DROP)\s+USER\s+MAPPING\b|tepp_app_runtime' --glob '*.sql' . || true

Length of output: 228832


@seonghobae, I reviewed exact head f5d3c956d6662bab7e64711d5e509917a73d0f8e.

I found no issue in the requested scope.

declares_created_role() now performs delimiter tokenization only after lexically_normalize_migration_sql() completes. The scan makes both ; and , standalone tokens. Therefore, this valid SQL produces a separate DROP token:

CREATE ROLE tepp_app_runtime;DROP ROLE tepp_app_runtime;

The lifecycle scan sets the role state at CREATE ROLE. It then clears the state at the adjacent DROP ROLE.

The lexical boundary still masks comments, single-quoted literals, quoted identifiers that contain unsupported characters, and dollar-quoted bodies before delimiter tokenization. A semicolon or comma in those regions cannot create a lifecycle token or a false role drop.

drop_statement_mentions_role() examines names until it reaches the standalone semicolon token. It continues across standalone comma tokens. This preserves detection for comma-separated role lists and prevents scanning into the next statement.

The existing DROP USER MAPPING disambiguation remains correct. is_role_drop_alias() rejects the USER MAPPING form before drop_statement_mentions_role() can change the runtime-role state.

The public RED covers adjacent DROP ROLE, DROP USER, and DROP GROUP statements through validate_migration_catalog(). The lower-level regression also verifies the adjacent DROP ROLE transition directly.

This was a source review only. I did not run build or test commands. Draft lifecycle results remain non-promotion evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head fd09a73c48ec033556bcf4d465fc9276a2bff450. New public RED 6d276133e45b4780d9756f263855e343abd0d159 covers PostgreSQL ALTER ROLE/ALTER USER/ALTER GROUP ... RENAME TO ... final-state invalidation and rename-target naming; causal fix fd09a73... tracks migration-established roles as an ordered set and projects rename targets through the existing naming scanner. Please focus on rename ordering, alias/user-mapping disambiguation, delimiter handling, and any false-positive/false-negative role lifecycle transitions. This PR remains Draft; skipped CI is diagnostic only, not promotion evidence.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='0790db3b27bb1eb12ec4cdaf11a78c66c96c8de6'
red='9b47d91c0e7996fef6973b4b1aaaa9c22b6f7b2b'
prior_repair='6de2b13dcd1432fc3c64eed44890763fcf836f21'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'
test='crates/persistence_postgres/tests/migration_unicode_set_config_identity_contract.rs'

printf '%s\n' '== exact head and ancestry =='
printf 'checkout='; git rev-parse HEAD
git diff --quiet "$head" HEAD && echo 'checkout-matches-requested-head'
for revision in "$prior_repair" "$red" "$head"; do
  git show --no-patch --format='revision=%H%nparents=%P%nsubject=%s' "$revision"
  git merge-base --is-ancestor "$revision" "$head" && echo 'is-ancestor=yes' || echo 'is-ancestor=no'
done
git log --oneline "${red}^..${head}"
git diff --check "${head}^" "$head"

printf '%s\n' '== scoped repair diff =='
git diff --name-status "$red" "$head" -- "$src" "$lex" "$projection" "$test"
git diff --unified=100 "$red" "$head" -- "$src" "$lex" "$projection" "$test"

printf '%s\n' '== source maps =='
ast-grep outline "$src" --items all --type function,struct,enum,impl || true
ast-grep outline "$lex" --items all --type function,struct,enum,impl || true
ast-grep outline "$projection" --items all --type function,struct,enum,impl || true

printf '%s\n' '== identity projector, set_config parser, and execution call flow =='
git show "${head}:${src}" > /tmp/migration_validation_587_rereview.rs
git show "${head}:${lex}" > /tmp/migration_validation_impl_587_rereview.rs
rg -n -C 70 -i \
 'fn (project_potential_unicode_set_config_identity|unicode_identifier_end|statement_calls_unsafe_set_config|sql_span_names_set_config|committed_replica_trigger_execution_mode|next_sql_token_span)|U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|set_config|pg_catalog|setting_name|new_value|is_local|session_replication_role|project_committed_sql' \
 /tmp/migration_validation_587_rereview.rs /tmp/migration_validation_impl_587_rereview.rs

printf '%s\n' '== focused production windows =='
nl -ba /tmp/migration_validation_587_rereview.rs | sed -n '1,340p'
nl -ba /tmp/migration_validation_587_rereview.rs | sed -n '720,830p'
nl -ba /tmp/migration_validation_impl_587_rereview.rs | sed -n '150,390p'
nl -ba /tmp/migration_validation_impl_587_rereview.rs | sed -n '760,880p'
git show "${head}:${projection}" | nl -ba | sed -n '1,230p'

printf '%s\n' '== public scoped contract =='
git show "${head}:${test}" | nl -ba

printf '%s\n' '== relevant controls across exact head =='
git grep -n -C 12 -i -E \
 'set_config|pg_catalog|U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|setting_name|new_value|is_local|session_replication_role|replica|origin|local|audit_support|application_name|ROLLBACK|ABORT|comment|literal|dollar' \
 "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests/*.rs

printf '%s\n' '== source-only structural checks =='
python3 - <<'PY'
import subprocess
head='0790db3b27bb1eb12ec4cdaf11a78c66c96c8de6'
for path in (
    'crates/persistence_postgres/src/migration_validation.rs',
    'crates/persistence_postgres/src/migration_validation_impl.rs',
    'crates/persistence_postgres/src/migration_transaction_projection.rs',
):
    source = subprocess.check_output(['git', 'show', f'{head}:{path}'], text=True)
    print(f'{path}: brace_delta={source.count("{") - source.count("}")}')
PY

Length of output: 204003


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='0790db3b27bb1eb12ec4cdaf11a78c66c96c8de6'
red='9b47d91c0e7996fef6973b4b1aaaa9c22b6f7b2b'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'
test='crates/persistence_postgres/tests/migration_unicode_set_config_identity_contract.rs'

echo '== exact change =='
git diff --unified=45 "$red" "$head" -- "$src" "$test"

echo '== production symbols =='
git show "${head}:${src}" > /tmp/mv.rs
for symbol in \
  project_potential_unicode_set_config_identity \
  unicode_identifier_end \
  statement_calls_unsafe_set_config \
  sql_span_names_set_config \
  committed_replica_trigger_execution_mode \
  declares_created_role; do
  line=$(rg -n -m1 "^fn ${symbol}\b" /tmp/mv.rs | cut -d: -f1 || true)
  printf '\n--- %s (%s) ---\n' "$symbol" "${line:-missing}"
  if [ -n "$line" ]; then
    start=$((line > 15 ? line - 15 : 1))
    end=$((line + 150))
    nl -ba /tmp/mv.rs | sed -n "${start},${end}p"
  fi
done

echo '== lexical Unicode and literal output rules =='
git show "${head}:${lex}" > /tmp/mvl.rs
rg -n -C 35 'fn (scan_unicode_escaped_identifier|scan_quoted_identifier|literal_is_atomic|normalize_migration_sql)|INVALID_QUOTED_IDENTIFIER|UESCAPE' /tmp/mvl.rs

echo '== transaction projection =='
git show "${head}:${projection}" | nl -ba | sed -n '1,210p'

echo '== complete scoped public contract =='
git show "${head}:${test}" | nl -ba

echo '== exact source occurrences relevant to UESCAPE identity projection =='
rg -n -C 10 'UESCAPE|unicode_identifier_end|project_potential_unicode_set_config_identity|INVALID_QUOTED_IDENTIFIER' /tmp/mv.rs

echo '== diff whitespace =='
git diff --check "${head}^" "$head"

Length of output: 76905


@seonghobae I re-reviewed exact head 0790db3b27bb1eb12ec4cdaf11a78c66c96c8de6 for #587.

The checkout matches the requested head. Public RED 9b47d91c0e7996fef6973b4b1aaaa9c22b6f7b2b is an ancestor.

I found no source-level bypass or false positive in the requested scope.

project_potential_unicode_set_config_identity() now consumes UESCAPE after a structural U& identifier. It consumes the following quoted token only when that token survived lexical normalization. This supports both normalized forms:

  • A retained atomic escape literal, such as UESCAPE '_'.
  • A marker-only form after the lexer masks a non-atomic literal, such as UESCAPE '!'.

For the prior failing form:

SELECT U&"set_!0063onfig" UESCAPE '!'(
    setting_name => 'session_replication_role',
    new_value => 'replica',
    is_local => false
);

the projector now leaves ( as the token after the projected canonical identity. statement_calls_unsafe_set_config() then detects the builtin call and processes the named arguments. The unsafe value returns MissingAppRuntimeRole.

The same path preserves direct safe values. The origin case in migration_unicode_set_config_identity_contract.rs remains a canonical call with a statically safe new_value.

The bounded identity rules remain intact:

  • Exact or unresolved canonical U& projections for set_config and pg_catalog fail closed.
  • A dual Unicode-escaped pg_catalog.set_config form projects to the canonical builtin path.
  • Safely projected unrelated names, including U&"audit_set_config" and U&"audit_support".U&"set_config", do not satisfy the canonical-identity condition.
  • The named-argument parser runs after identity projection. It retains the existing positional, named, and mixed notation handling.

declares_created_role() still applies shared lexical normalization before committed-state projection and the execution-mode check. Therefore, comments, string literals, and dollar-quoted bodies remain opaque. Statements in transactions ending with ROLLBACK or ABORT remain excluded before set_config inspection.

git diff --check reported no whitespace errors. This is exact-head source-review evidence only. I did not build, run tests, or execute PostgreSQL. Draft-skipped Actions are not execution or promotion evidence.


✏️ Learnings added
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T12:11:14.621Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `project_potential_unicode_set_config_identity()` runs after shared lexical normalization. When normalized `UESCAPE` is followed by a non-atomic one-character literal such as `'!'`, the lexer masks that literal. The identity projector must consume `UESCAPE` and accept this marker-only representation, while consuming a following quoted token only when an atomic escape literal was retained. This preserves canonical Unicode-escaped `set_config` detection without reimplementing raw SQL literal parsing.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T10:14:43.914Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `update_targets_unsafe_replication_role_via_pg_settings()` must treat a structurally projected Unicode-escaped `U&"pg_catalog"` schema as canonical when it qualifies `pg_settings`. It must fail closed for `INVALID_QUOTED_IDENTIFIER` projections and for any remaining structural `U&` marker after canonical `pg_settings` identity is established. This prevents Unicode-escaped relation, alias, or assignment-column identities from bypassing the committed replica execution-policy check until the shared lexer owns Unicode escape decoding.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T09:08:08.633Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, a committed PostgreSQL top-level `DO` statement must fail closed in the runtime execution-context check. PostgreSQL executes the anonymous procedural body immediately, but the shared lexical normalizer intentionally makes procedural bodies opaque. The check must run after committed-statement projection so a `DO` statement in a transaction that ends with `ROLLBACK` or `ABORT` is discarded. `CREATE FUNCTION` and `CREATE PROCEDURE` bodies are not immediate `DO` execution because their structural statement token is `CREATE`.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-17T03:01:59.380Z
Learning: For PR `#521`, Draft or lifecycle-skipped CI is not GREEN and is not promotion evidence. Migration validator review conclusions require exact-head source or executed-gate evidence.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 8c2d868031801874a0fb09108417ba7502138e3d, scoped to #588. Public RED lineage is 9fdd078f7459a3d1e468f50846d44c43038f4c6a (persistent ALTER ROLE/DATABASE/SYSTEM defaults) and supplemental alias RED d1088e7ed81595665c296c4151c3800e1d42162c (PostgreSQL 18 ALTER USER alias, including IN DATABASE, ALL, FROM CURRENT, safe and unrelated controls). Repairs are 7cf0f42eda53d0bc6452bafc6fc69f4b8afadbfb and current 8c2d868....

Please focus on whether normalized+committed SQL can still leave a persistent session_replication_role default that makes a later TEPP runtime session start in replica mode, or whether this bounded repair rejects unrelated/safe configuration incorrectly. Check ALTER ROLE/USER target identity, USER MAPPING disambiguation, IN DATABASE, ALL, ALTER DATABASE, ALTER SYSTEM, FROM CURRENT, origin/local, rollback projection, and marker opacity. Also flag if the conservative RESET/DEFAULT treatment creates a scientifically meaningful final-state false positive that should be repaired now rather than deferred to the first-class default-state aggregate.

This remains Draft. Hosted Rust/Live PostgreSQL gates are skipped by lifecycle, so this request is source-review evidence only and must not be treated as GREEN or qualifying approval.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='8c2d868031801874a0fb09108417ba7502138e3d'
red1='9fdd078f7459a3d1e468f50846d44c43038f4c6a'
red2='d1088e7ed81595665c296c4151c3800e1d42162c'
repair1='7cf0f42eda53d0bc6452bafc6fc69f4b8afadbfb'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'

printf '%s\n' '== exact revision and lineage =='
printf 'checkout='; git rev-parse HEAD
git diff --quiet "$head" HEAD && echo 'checkout-matches-requested-head'
for revision in "$red1" "$repair1" "$red2" "$head"; do
  git show --no-patch --format='revision=%H%nparents=%P%nsubject=%s' "$revision"
  git merge-base --is-ancestor "$revision" "$head" && echo 'is-ancestor=yes' || echo 'is-ancestor=no'
done
git log --oneline "${red1}^..${head}"
git diff --check "${head}^" "$head"

printf '%s\n' '== scoped changes =='
git diff --name-status "${red1}^" "$head" -- crates/persistence_postgres/src crates/persistence_postgres/tests
git diff --unified=85 "${red1}^" "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests

printf '%s\n' '== source maps =='
for file in "$src" "$lex" "$projection"; do
  echo "--- $file ---"
  ast-grep outline "$file" --items all --type function,struct,enum,impl || true
done

printf '%s\n' '== persistent setting authority and callers =='
git show "${head}:${src}" > /tmp/migration_validation_588.rs
rg -n -C 80 -i \
  'fn .*persistent|fn .*default|fn .*alter|session_replication_role|ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|USER[[:space:]]+MAPPING|IN[[:space:]]+DATABASE|FROM[[:space:]]+CURRENT|RESET|DEFAULT|origin|local|replica|project_committed_sql|committed_replica_trigger_execution_mode|declares_created_role' \
  /tmp/migration_validation_588.rs

printf '%s\n' '== focused production source =='
nl -ba /tmp/migration_validation_588.rs | sed -n '1,950p'
printf '%s\n' '== lexical authority =='
git show "${head}:${lex}" | nl -ba | sed -n '1,430p'
printf '%s\n' '== transaction projection =='
git show "${head}:${projection}" | nl -ba | sed -n '1,270p'

printf '%s\n' '== relevant public contracts =='
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests |
  rg -i 'migration.*(persistent|default|alter|session.*replication|runtime.*role|transaction).*\.rs$' |
  while IFS= read -r file; do
    echo "--- $file ---"
    git show "${head}:${file}" | nl -ba
  done

printf '%s\n' '== all scoped forms =='
git grep -n -C 15 -i -E \
  'ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|USER[[:space:]]+MAPPING|IN[[:space:]]+DATABASE|FROM[[:space:]]+CURRENT|session_replication_role|RESET|DEFAULT|origin|local|replica|ROLLBACK|ABORT|BEGIN|COMMIT|comment|literal|dollar' \
  "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests/*.rs

printf '%s\n' '== structural source check =='
python3 - <<'PY'
import subprocess
head='8c2d868031801874a0fb09108417ba7502138e3d'
for path in (
    'crates/persistence_postgres/src/migration_validation.rs',
    'crates/persistence_postgres/src/migration_validation_impl.rs',
    'crates/persistence_postgres/src/migration_transaction_projection.rs',
):
    source=subprocess.check_output(['git','show',f'{head}:{path}'], text=True)
    print(f'{path}: brace_delta={source.count("{") - source.count("}")}')
PY

Length of output: 203889


🤖 web_search executed:

PostgreSQL 18 official documentation ALTER ROLE SET, ALTER USER SET, ALTER DATABASE SET, ALTER SYSTEM SET, RESET, DEFAULT, FROM CURRENT, IN DATABASE, and whether ALTER ROLE ALL SET affects future session configuration defaults.

Length of output: 14023


🏁 Script executed:

#!/bin/bash
set -euo pipefail
head='8c2d868031801874a0fb09108417ba7502138e3d'
src='crates/persistence_postgres/src/migration_validation.rs'

git show "${head}:${src}" > /tmp/mv588.rs

echo '== relevant function locations =='
rg -n '^\s*(pub\(super\)\s+)?fn\s+.*(persistent|default|alter|replica|configuration|setting)|persistent.*replica|ALTER SYSTEM|ALTER DATABASE|ALTER ROLE|ALTER USER|FROM CURRENT|USER MAPPING' /tmp/mv588.rs

echo '== focused persistent-default implementation =='
for name in $(rg -o '^\s*fn\s+[A-Za-z0-9_]+' /tmp/mv588.rs | sed -E 's/.*fn\s+//' | rg -i 'persistent|default|alter|configuration|setting|replica' | sort -u); do
  line=$(rg -n -m1 "^\s*fn ${name}\b" /tmp/mv588.rs | cut -d: -f1)
  echo "--- $name at $line ---"
  nl -ba /tmp/mv588.rs | sed -n "$(( line > 20 ? line - 20 : 1 )),$((line+200))p"
done

echo '== execution-validation call order =='
rg -n -C 25 'project_committed_sql|committed_replica_trigger_execution_mode|persistent.*replica|declares_created_role' /tmp/mv588.rs

echo '== changed tests since alias RED =='
git diff --name-status d1088e7ed81595665c296c4151c3800e1d42162c "$head" -- crates/persistence_postgres/tests crates/persistence_postgres/src/migration_validation.rs
git diff --unified=80 d1088e7ed81595665c296c4151c3800e1d42162c "$head" -- crates/persistence_postgres/tests crates/persistence_postgres/src/migration_validation.rs

echo '== exact `#588` test candidates =='
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests | rg -i '(persistent|default|alter.*(role|user|database|system)|replication.*default).*\.rs$' | while read -r f; do
  echo "--- $f ---"
  git show "${head}:${f}" | nl -ba
done

echo '== direct context around RESET DEFAULT FROM CURRENT =='
rg -n -C 45 'RESET|DEFAULT|FROM CURRENT|IN DATABASE|USER MAPPING|ALTER SYSTEM|ALTER DATABASE|ALTER USER|ALTER ROLE' /tmp/mv588.rs

git diff --check "${head}^" "$head"

Length of output: 112436


🏁 Script executed:

#!/bin/bash
set -euo pipefail
head='8c2d868031801874a0fb09108417ba7502138e3d'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'

git show "${head}:${src}" > /tmp/mv588_edge.rs
git show "${head}:${lex}" > /tmp/mvl588_edge.rs

echo '== quoted-identifier normalization =='
rg -n -C 55 'fn scan_quoted_identifier|fn quoted_identifier|INVALID_QUOTED_IDENTIFIER|quoted.*identity|to_ascii_lowercase|normalize_migration_sql' /tmp/mvl588_edge.rs

echo '== exact persistent parser and preceding token helpers =='
nl -ba /tmp/mv588_edge.rs | sed -n '670,820p'
rg -n -C 25 'fn is_word_start|fn identifier_continues_after|fn next_sql_token_span|fn .*token' /tmp/mv588_edge.rs

echo '== all persistent-default tests and source cases =='
git grep -n -C 10 -i -E \
 'ALTER (ROLE|USER|DATABASE|SYSTEM).*(session_replication_role|RESET|DEFAULT|FROM CURRENT)|USER MAPPING' \
 "$head" -- crates/persistence_postgres/tests '*.rs'

echo '== exact current commit =='
git show --format=fuller --stat --oneline "$head"
git diff --unified=100 "${head}^" "$head" -- crates/persistence_postgres/src/migration_validation.rs crates/persistence_postgres/tests

echo '== syntax variants represented in parser =='
python3 - <<'PY'
# Static token trace for variant shapes against the inspected parser's tokenization rule.
cases = [
 "ALTER ROLE tepp_app_runtime SET session_replication_role = replica",
 'ALTER ROLE "tepp_app_runtime" SET session_replication_role = replica',
 "ALTER ROLE ALL IN DATABASE tepp_database RESET ALL",
 "ALTER USER MAPPING FOR tepp_app_runtime SERVER foreign_server OPTIONS (SET user 'replica')",
 "ALTER SYSTEM RESET session_replication_role",
]
for sql in cases:
    print(sql)
    print(sql.replace('=', ' = ').split())
PY

git diff --check "${head}^" "$head"

Length of output: 121092


@seonghobae I reviewed #588 at exact head 8c2d868031801874a0fb09108417ba7502138e3d.

The checkout matches the requested head. Public RED commits 9fdd078f7459a3d1e468f50846d44c43038f4c6a and d1088e7ed81595665c296c4151c3800e1d42162c are ancestors.

I found no source-level bypass or false positive in the requested scope.

statement_sets_unsafe_persistent_replication_role_default() runs after shared lexical normalization and project_committed_sql(). Therefore, comment text, string literals, and dollar-quoted bodies are opaque. Statements in a transaction that ends with ROLLBACK or ABORT do not reach the persistent-default check.

The detector handles these persistent configuration surfaces:

  • ALTER ROLE
  • ALTER USER
  • ALTER DATABASE
  • ALTER SYSTEM

For role-scoped commands, it treats tepp_app_runtime, ALL, CURRENT_ROLE, CURRENT_USER, and SESSION_USER as runtime-relevant targets. This covers defaults that can affect the runtime role or all roles. The ALTER USER path now has the same treatment as ALTER ROLE.

The parser handles IN DATABASE for ALTER ROLE and ALTER USER. It rejects malformed IN DATABASE structure. It also does not misclassify ALTER USER MAPPING, because MAPPING is not a runtime-relevant target.

The detector accepts only direct origin and local values as safe:

ALTER USER tepp_app_runtime SET session_replication_role = origin;
ALTER ROLE ALL IN DATABASE tepp_database SET session_replication_role = local;
ALTER DATABASE tepp_database SET session_replication_role = local;
ALTER SYSTEM SET session_replication_role = origin;

It does not classify unrelated role targets or unrelated parameters as protected mutations. The public controls cover audit_runtime, application_name, and ALTER USER MAPPING.

FROM CURRENT, DEFAULT, and RESET remain fail-closed for the protected parameter or ALL. This is appropriate for the current bounded validator. PostgreSQL removes the local override for DEFAULT and RESET, then resolves the new-session value from the remaining precedence chain. The validator does not model that chain or external cluster configuration. Accepting these forms would create a bypass if an inherited role, database, or system default is replica.

This conservative behavior can reject a migration where the inherited value is known externally to be safe. That is not a scientifically meaningful false positive to repair in this change. The validator cannot prove the final value without a first-class default-state aggregate and explicit scope-precedence modeling. The current fail-closed behavior is correct until that aggregate exists.

I applied the existing PR guidance that Draft-skipped Actions are not execution or promotion evidence. This is exact-head source-review evidence only. I did not build, run tests, or execute PostgreSQL.


🧠 Learnings used
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T07:10:14.777Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, a committed `SET LOCAL session_replication_role = replica` is intentionally rejected as a fail-closed execution-policy violation. Although PostgreSQL resets `SET LOCAL` at transaction end, a committed transaction can execute protected DML while ordinary TEPP enforcement triggers are suppressed. Rolled-back replica-mode statements remain discarded by the shared transaction projection.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T06:10:55.597Z
Learning: For TEPP migration validation, `SET LOCAL session_replication_role = replica` is transaction-scoped and does not persist after `COMMIT` or `ROLLBACK`. In `crates/persistence_postgres/src/migration_validation.rs`, runtime-safety validation must reject committed session-level `SET` or `SET SESSION` changes to `replica`, but must not reject a completed `SET LOCAL` change solely as a durable final-state hazard.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-17T03:01:59.380Z
Learning: For PR `#521`, Draft or lifecycle-skipped CI is not GREEN and is not promotion evidence. Migration validator review conclusions require exact-head source or executed-gate evidence.

You are interacting with an AI system.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head source review for 8c2d868031801874a0fb09108417ba7502138e3d: #588 adds a persistent PostgreSQL execution-context boundary that was absent from #576#587. PostgreSQL 18 applies ALTER ROLE / ALTER USER and ALTER DATABASE configuration values when fresh sessions start, and ALTER SYSTEM writes cluster defaults to postgresql.auto.conf; session_replication_role=replica suppresses ordinary/default triggers and rules. Public RED 9fdd078f7459a3d1e468f50846d44c43038f4c6a covers role/database/system replica defaults, FROM CURRENT, origin/local, unrelated targets, rollback, and opaque marker text. First repair 7cf0f42... adds a documented fold after the shared lexical + committed-state projection. PostgreSQL 18 documents ALTER USER as an alias for ALTER ROLE with the same {role_specification | ALL} [IN DATABASE ...] SET/RESET grammar; supplemental public RED d1088e7ed81595665c296c4151c3800e1d42162c exposed that omitted sibling, and current repair 8c2d868... folds USER onto the same authority while keeping ALTER USER MAPPING unrelated. Direct origin/local remain ordinary-trigger-safe. No executable migration SQL, workflow, dependency, threshold, coverage denominator, skip/xfail, or gate changed. Known bounded limitation is explicit: RESET/DEFAULT/FROM CURRENT and precedence are conservatively fail-closed until a first-class role/role+database/database/system default-state aggregate can prove final inherited state. This COMMENT is source review only, not approval or hosted GREEN; exact-head Rust Foundation 35349155710 is Draft-skipped.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head 41f9cb437ee5ae114b46e15ab2ed2ff8d957be43 for #589 only. Public RED ancestry is 64735626dd86616020cdb60b90028adbf1d2a70a then supplemental RESET controls f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8; causal repair is exact 41f9cb437ee5ae114b46e15ab2ed2ff8d957be43.

Focus on PostgreSQL normalized U& identity for persistent ALTER ROLE/ALTER USER runtime-role targets and session_replication_role parameters across role/user/database/system SET/RESET, optional UESCAPE, safe origin/local, unrelated Unicode identities, transaction rollback, and opaque regions. Please flag any source-level bypass/false positive or Rust compile issue. This is source review only; do not treat it as hosted GREEN or a qualifying approval.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='41f9cb437ee5ae114b46e15ab2ed2ff8d957be43'
red1='64735626dd86616020cdb60b90028adbf1d2a70a'
red2='f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8'
validation='crates/persistence_postgres/src/migration_validation.rs'
lexical='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'

printf '%s\n' '== exact revision and RED ancestry =='
printf 'checkout='; git rev-parse HEAD
git diff --quiet "$head" HEAD && echo 'checkout-matches-requested-head'
for revision in "$red1" "$red2" "$head"; do
  git show --no-patch --format='revision=%H%nparents=%P%nsubject=%s' "$revision"
  git merge-base --is-ancestor "$revision" "$head" && echo 'is-ancestor=yes' || echo 'is-ancestor=no'
done
git log --oneline "${red1}^..${head}"
git diff --check "${head}^" "$head"

printf '%s\n' '== scoped repair changes =='
git diff --name-status "$red1" "$head" -- crates/persistence_postgres/src crates/persistence_postgres/tests
git diff --unified=100 "$red2" "$head" -- "$validation" "$lexical" "$projection" crates/persistence_postgres/tests

printf '%s\n' '== source outlines =='
for file in "$validation" "$lexical" "$projection"; do
  printf '\n--- %s ---\n' "$file"
  ast-grep outline "$file" --items all --type function,struct,enum,impl || true
done

printf '%s\n' '== persistent-default parser and validation order =='
git show "${head}:${validation}" > /tmp/migration_validation_589.rs
git show "${head}:${lexical}" > /tmp/migration_validation_impl_589.rs
rg -n -C 90 -i \
  'fn .*persistent|fn .*unicode|fn .*set.*replica|ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|USER[[:space:]]+MAPPING|U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|session_replication_role|RESET|DEFAULT|FROM[[:space:]]+CURRENT|origin|local|project_committed_sql|committed_replica_trigger_execution_mode|declares_created_role' \
  /tmp/migration_validation_589.rs /tmp/migration_validation_impl_589.rs

printf '%s\n' '== focused source windows =='
nl -ba /tmp/migration_validation_589.rs | sed -n '1,1000p'
git show "${head}:${projection}" | nl -ba | sed -n '1,300p'
nl -ba /tmp/migration_validation_impl_589.rs | sed -n '1,430p'

printf '%s\n' '== relevant public contracts =='
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests |
  rg -i 'migration.*(unicode.*(persistent|default|alter)|persistent.*(unicode|default|alter)|session.*replication|alter.*(role|user|database|system)|transaction).*\.rs$' |
  while IFS= read -r file; do
    printf '\n--- %s ---\n' "$file"
    git show "${head}:${file}" | nl -ba
  done

printf '%s\n' '== exact scoped forms in source and tests =='
git grep -n -C 18 -i -E \
  'ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|USER[[:space:]]+MAPPING|IN[[:space:]]+DATABASE|FROM[[:space:]]+CURRENT|U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|session_replication_role|RESET|DEFAULT|origin|local|ROLLBACK|ABORT|BEGIN|COMMIT|comment|literal|dollar' \
  "$head" -- "$validation" "$lexical" "$projection" crates/persistence_postgres/tests/*.rs

printf '%s\n' '== Rust source-level sanity checks =='
python3 - <<'PY'
import subprocess
head='41f9cb437ee5ae114b46e15ab2ed2ff8d957be43'
for path in (
    'crates/persistence_postgres/src/migration_validation.rs',
    'crates/persistence_postgres/src/migration_validation_impl.rs',
    'crates/persistence_postgres/src/migration_transaction_projection.rs',
):
    source = subprocess.check_output(['git', 'show', f'{head}:{path}'], text=True)
    print(f'{path}: brace_delta={source.count("{") - source.count("}")}')
PY

Length of output: 204094


🏁 Script executed:

#!/bin/bash
set -euo pipefail
head='41f9cb437ee5ae114b46e15ab2ed2ff8d957be43'
red='f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'
test='crates/persistence_postgres/tests/migration_unicode_replication_role_login_default_contract.rs'

git show "${head}:${src}" > /tmp/mv589.rs
git show "${head}:${lex}" > /tmp/mvl589.rs

echo '== exact `#589` diff =='
git diff --unified=80 "$red" "$head" -- "$src" "$test"

echo '== persistent-parser symbols =='
rg -n '^\s*(pub\(super\)\s+)?fn\s+(statement_sets_unsafe_persistent_replication_role_default|.*persistent.*|.*unicode.*identity|unicode_identifier_end|project_potential_unicode.*|committed_replica_trigger_execution_mode|declares_created_role|next_sql_token_span)' /tmp/mv589.rs

echo '== persistent parser source =='
line=$(rg -n -m1 '^\s*fn\s+statement_sets_unsafe_persistent_replication_role_default\b' /tmp/mv589.rs | cut -d: -f1)
nl -ba /tmp/mv589.rs | sed -n "$((line-80)),$((line+360))p"

echo '== relevant Unicode helper source =='
rg -n -C 50 'UESCAPE|INVALID_QUOTED_IDENTIFIER|unicode_identifier_end|project_potential_unicode|fn scan_unicode_escaped_identifier|fn literal_is_atomic' /tmp/mv589.rs /tmp/mvl589.rs

echo '== caller and transaction projection =='
rg -n -C 25 'project_committed_sql|statement_sets_unsafe_persistent_replication_role_default|committed_replica_trigger_execution_mode|declares_created_role' /tmp/mv589.rs
git show "${head}:${projection}" | nl -ba | sed -n '1,230p'

echo '== scoped public contract =='
git show "${head}:${test}" | nl -ba

echo '== source-level type references in changed region =='
git diff --unified=25 "$red" "$head" -- "$src" |
  rg -n -C 12 '^[+-].*(Vec|Option|Result|Iterator|split|find|map|matches|return|Some|None|U&|UESCAPE|INVALID)' || true

echo '== whitespace =='
git diff --check "${head}^" "$head"

Length of output: 154070


@seonghobae I reviewed exact head 41f9cb437ee5ae114b46e15ab2ed2ff8d957be43 for #589.

The checkout matches the requested head. Public RED revisions 64735626dd86616020cdb60b90028adbf1d2a70a and f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8 are ancestors.

I found one source-level false positive.

preserve_quoted_special_role_specifications() in crates/persistence_postgres/src/migration_validation.rs, Lines 1033-1036, rewrites quoted special-role spellings before shared lexical normalization. The global replacement also changes the payload of a Unicode-escaped quoted identifier.

This valid statement targets the named role current_user. It does not target the executor-relative CURRENT_USER pseudo-role or tepp_app_runtime:

ALTER ROLE U&"current_user"
SET session_replication_role = replica;

Before lexical normalization, preserve_quoted_special_role_specifications() changes the identifier to:

U&"CURRENT_USER"

The shared lexical projection then emits INVALID_QUOTED_IDENTIFIER because the spelling is uppercase. In statement_sets_unsafe_persistent_replication_role_default(), Lines 753-766, INVALID_QUOTED_IDENTIFIER in the role-target slot is treated as potentially tepp_app_runtime. The protected parameter and value then cause MissingAppRuntimeRole.

This rejects an unrelated role default.

The same false positive applies to:

ALTER USER U&"current_role"
SET session_replication_role = replica;

ALTER ROLE U&"session_user"
IN DATABASE tepp_database
RESET session_replication_role;

Add public controls for the three lowercase Unicode-escaped named roles. Each should return Ok(()).

Limit preserve_quoted_special_role_specifications() to ordinary double-quoted identifiers. It must not rewrite a quote that is immediately preceded by the U& Unicode-identifier prefix. An alternative is to preserve Unicode-escaped quoted role identity without changing its spelling before the shared lexical pass.

The scoped repair otherwise has the expected source-level behavior:

  • It recognizes canonical Unicode-escaped runtime-role and parameter identities.
  • It handles masked and retained UESCAPE literals.
  • It preserves direct origin and local.
  • It ignores safely projected unrelated Unicode parameters.
  • It applies committed-state projection before the persistent-default check.
  • It keeps comments, string literals, and dollar-quoted bodies opaque.

I found no Rust compile issue by source inspection. I did not build or run the code. This is source-review evidence only. Draft-skipped Actions are not hosted GREEN or qualifying approval.


🧠 Learnings used
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T12:11:14.723Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `project_potential_unicode_set_config_identity()` runs after shared lexical normalization. When normalized `UESCAPE` is followed by a non-atomic one-character literal such as `'!'`, the lexer masks that literal. The identity projector must consume `UESCAPE` and accept this marker-only representation, while consuming a following quoted token only when an atomic escape literal was retained. This preserves canonical Unicode-escaped `set_config` detection without reimplementing raw SQL literal parsing.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T10:14:43.914Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `update_targets_unsafe_replication_role_via_pg_settings()` must treat a structurally projected Unicode-escaped `U&"pg_catalog"` schema as canonical when it qualifies `pg_settings`. It must fail closed for `INVALID_QUOTED_IDENTIFIER` projections and for any remaining structural `U&` marker after canonical `pg_settings` identity is established. This prevents Unicode-escaped relation, alias, or assignment-column identities from bypassing the committed replica execution-policy check until the shared lexer owns Unicode escape decoding.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T09:08:08.633Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, a committed PostgreSQL top-level `DO` statement must fail closed in the runtime execution-context check. PostgreSQL executes the anonymous procedural body immediately, but the shared lexical normalizer intentionally makes procedural bodies opaque. The check must run after committed-statement projection so a `DO` statement in a transaction that ends with `ROLLBACK` or `ABORT` is discarded. `CREATE FUNCTION` and `CREATE PROCEDURE` bodies are not immediate `DO` execution because their structural statement token is `CREATE`.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T07:10:14.777Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, a committed `SET LOCAL session_replication_role = replica` is intentionally rejected as a fail-closed execution-policy violation. Although PostgreSQL resets `SET LOCAL` at transaction end, a committed transaction can execute protected DML while ordinary TEPP enforcement triggers are suppressed. Rolled-back replica-mode statements remain discarded by the shared transaction projection.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T06:10:55.597Z
Learning: For TEPP migration validation, `SET LOCAL session_replication_role = replica` is transaction-scoped and does not persist after `COMMIT` or `ROLLBACK`. In `crates/persistence_postgres/src/migration_validation.rs`, runtime-safety validation must reject committed session-level `SET` or `SET SESSION` changes to `replica`, but must not reject a completed `SET LOCAL` change solely as a durable final-state hazard.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-17T21:11:30.461Z
Learning: For TEPP migration validation, `crates/persistence_postgres/src/migration_runtime_role_executor.rs::transaction_end()` must treat PostgreSQL `ABORT` as the `ROLLBACK` alias. Otherwise `ABORT` leaves `SET LOCAL ROLE` or local session-authorization state in the executor projection, and a later `GRANTED BY CURRENT_USER` or `CURRENT_ROLE` can incorrectly revoke an unsafe explicit-grantor membership row.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please re-review exact head 6583ac5963665305ba08ab1ac2c5fd52b6450d22 for #589 only.

Lineage:

  • 64735626dd86616020cdb60b90028adbf1d2a70a — original public RED for Unicode persistent-default identities;
  • f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8 — supplemental Unicode RESET controls;
  • 41f9cb437ee5ae114b46e15ab2ed2ff8d957be43 — first repair;
  • your valid finding: lifecycle preprojection globally rewrote the payload inside U&"current_user", causing a named unrelated role false positive;
  • 5e356d68116dfb18294a487c505227470067ae0d — public RED/controls for U&"current_user", U&"current_role", and U&"session_user" named-role identities;
  • 6583ac5963665305ba08ab1ac2c5fd52b6450d22 — current causal repair, which leaves immediately U&-prefixed quoted special-role names untouched while preserving ordinary quoted special-role disambiguation.

Please check: (1) the reported false positive is actually removed; (2) ordinary "current_user"/"current_role"/"session_user" still cannot alias executor-relative pseudo-targets; (3) protected U&"tepp_app_runtime" and Unicode session_replication_role SET/RESET paths remain fail-closed; (4) immediate U& prefix handling does not introduce a source-level bypass or false positive; (5) Rust source compiles structurally. This remains source-review evidence only, not hosted GREEN or qualifying approval.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='6583ac5963665305ba08ab1ac2c5fd52b6450d22'
red1='64735626dd86616020cdb60b90028adbf1d2a70a'
red2='f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8'
prior='41f9cb437ee5ae114b46e15ab2ed2ff8d957be43'
red3='5e356d68116dfb18294a487c505227470067ae0d'
src='crates/persistence_postgres/src/migration_validation.rs'
lex='crates/persistence_postgres/src/migration_validation_impl.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'

printf '%s\n' '== exact revision and lineage =='
printf 'checkout='; git rev-parse HEAD
git diff --quiet "$head" HEAD && echo 'checkout-matches-requested-head'
for revision in "$red1" "$red2" "$prior" "$red3" "$head"; do
  git show --no-patch --format='revision=%H%nparents=%P%nsubject=%s' "$revision"
  git merge-base --is-ancestor "$revision" "$head" && echo 'is-ancestor=yes' || echo 'is-ancestor=no'
done
git log --oneline "${red1}^..${head}"
git diff --check "${head}^" "$head"

printf '%s\n' '== focused current repair diff =='
git diff --name-status "$red3" "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests
git diff --unified=100 "$red3" "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests

printf '%s\n' '== source maps =='
for file in "$src" "$lex" "$projection"; do
  echo "--- $file ---"
  ast-grep outline "$file" --items all --type function,struct,enum,impl || true
done

printf '%s\n' '== role preprojection, persistent defaults, unicode paths, and call flow =='
git show "${head}:${src}" > /tmp/migration_validation_589_rereview.rs
git show "${head}:${lex}" > /tmp/migration_validation_impl_589_rereview.rs
rg -n -C 80 -i \
  'fn (preserve_quoted_special_role_specifications|statement_sets_unsafe_persistent_replication_role_default|project_potential_unicode|unicode_identifier_end|committed_replica_trigger_execution_mode|declares_created_role|next_sql_token_span)|CURRENT_USER|CURRENT_ROLE|SESSION_USER|tepp_app_runtime|session_replication_role|ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|RESET|origin|local|project_committed_sql' \
  /tmp/migration_validation_589_rereview.rs /tmp/migration_validation_impl_589_rereview.rs

printf '%s\n' '== focused production ranges =='
nl -ba /tmp/migration_validation_589_rereview.rs | sed -n '680,1120p'
nl -ba /tmp/migration_validation_impl_589_rereview.rs | sed -n '150,390p'
git show "${head}:${projection}" | nl -ba | sed -n '1,240p'

printf '%s\n' '== relevant public contracts =='
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests |
  rg -i 'migration.*(unicode.*(replication|default|login|role)|replication.*(unicode|default|login)|persistent|session.*replication|transaction).*\.rs$' |
  while IFS= read -r file; do
    echo "--- $file ---"
    git show "${head}:${file}" | nl -ba
  done

printf '%s\n' '== all exact scoped controls =='
git grep -n -C 14 -i -E \
  'U&|UESCAPE|INVALID_QUOTED_IDENTIFIER|CURRENT_USER|CURRENT_ROLE|SESSION_USER|tepp_app_runtime|session_replication_role|ALTER[[:space:]]+(ROLE|USER|DATABASE|SYSTEM)|RESET|DEFAULT|origin|local|ROLLBACK|ABORT|comment|literal|dollar' \
  "$head" -- "$src" "$lex" "$projection" crates/persistence_postgres/tests/*.rs

printf '%s\n' '== structural source checks =='
python3 - <<'PY'
import subprocess
head='6583ac5963665305ba08ab1ac2c5fd52b6450d22'
for path in (
  'crates/persistence_postgres/src/migration_validation.rs',
  'crates/persistence_postgres/src/migration_validation_impl.rs',
  'crates/persistence_postgres/src/migration_transaction_projection.rs',
):
    source = subprocess.check_output(['git', 'show', f'{head}:{path}'], text=True)
    print(f'{path}: brace_delta={source.count("{") - source.count("}")}')
PY

Length of output: 204266


🏁 Script executed:

#!/bin/bash
set -euo pipefail
head='6583ac5963665305ba08ab1ac2c5fd52b6450d22'
red='5e356d68116dfb18294a487c505227470067ae0d'
src='crates/persistence_postgres/src/migration_validation.rs'
test='crates/persistence_postgres/tests/migration_unicode_replication_role_login_default_contract.rs'

git show "${head}:${src}" > /tmp/mv589_final.rs

echo '== exact head and ancestry =='
git rev-parse HEAD
git diff --quiet "$head" HEAD && echo checkout-matches-requested-head
for revision in 64735626dd86616020cdb60b90028adbf1d2a70a f4e2aabbd844bfd16c3f83a4d18b1253bf60fdc8 41f9cb437ee5ae114b46e15ab2ed2ff8d957be43 "$red"; do
  git merge-base --is-ancestor "$revision" "$head" && echo "$revision ancestor=yes" || echo "$revision ancestor=no"
done

echo '== current repair diff =='
git diff --unified=45 "$red" "$head" -- "$src" "$test"

echo '== function locations =='
rg -n '^\s*fn\s+(preserve_quoted_special_role_specifications|statement_sets_unsafe_persistent_replication_role_default|unicode_identifier_end|project_potential_unicode|committed_replica_trigger_execution_mode|declares_created_role)\b' /tmp/mv589_final.rs

echo '== special-role preprojection and caller context =='
line=$(rg -n -m1 '^\s*fn\s+preserve_quoted_special_role_specifications\b' /tmp/mv589_final.rs | cut -d: -f1)
nl -ba /tmp/mv589_final.rs | sed -n "$((line-35)),$((line+145))p"

echo '== persistent-default parser =='
line=$(rg -n -m1 '^\s*fn\s+statement_sets_unsafe_persistent_replication_role_default\b' /tmp/mv589_final.rs | cut -d: -f1)
nl -ba /tmp/mv589_final.rs | sed -n "$((line-55)),$((line+335))p"

echo '== public unicode persistent-default contract =='
git show "${head}:${test}" | nl -ba

echo '== Rust syntax-oriented static scan =='
rg -n -C 3 'preserve_quoted_special_role_specifications|replace\(|strip_prefix|starts_with|unwrap\(|expect\(' /tmp/mv589_final.rs
git diff --check "${head}^" "$head"

Length of output: 50800


@seonghobae I re-reviewed exact head 6583ac5963665305ba08ab1ac2c5fd52b6450d22 for #589 only.

The checkout matches the requested head. The listed RED revisions and prior repair are ancestors of this head.

I found no source-level bypass, false positive, or structural Rust issue in the requested scope.

preserve_quoted_special_role_specifications() now checks the two bytes immediately before each matched quoted spelling. If they are U& or u&, it keeps the Unicode-escaped identifier unchanged. This removes the reported false positive:

ALTER ROLE U&"current_user"
SET session_replication_role = replica;

The shared lexical pass can now project this as the named lowercase role current_user. The persistent-default parser does not classify it as CURRENT_USER or as an invalid runtime-role identity. The public controls also cover U&"current_role" and U&"session_user".

For ordinary quoted identifiers, the preprojection still converts:

"current_user"
"current_role"
"session_user"

to case-distinct quoted spellings before lifecycle normalization. These identifiers therefore cannot alias the unquoted PostgreSQL pseudo-targets CURRENT_USER, CURRENT_ROLE, and SESSION_USER.

The persistent-default parser still fails closed for protected Unicode identities:

  • U&"tepp_app_runtime" remains runtime-relevant.
  • Unicode session_replication_role remains protected on ALTER ROLE, ALTER USER, ALTER DATABASE, and ALTER SYSTEM.
  • SET, RESET, and unresolved INVALID_QUOTED_IDENTIFIER forms retain fail-closed behavior.
  • UESCAPE consumes either a retained atomic literal or the marker-only representation after lexical masking.
  • Direct origin and local remain safe.

Safely projected unrelated Unicode role and parameter identities remain accepted. The public contract includes U&"audit_runtime", the three Unicode named special-role controls, and U&"application_name".

The lifecycle path still normalizes SQL before applying committed-state projection. Comments, string literals, and dollar-quoted bodies remain opaque. Statements in a transaction that ends with ROLLBACK or ABORT do not reach the persistent-default check.

The focused diff has balanced delimiters. git diff --check produced no whitespace errors. I did not build or run the code. This is source-review evidence only. As requested, it is not hosted GREEN or qualifying approval.

I used the existing migration-validation guidance for fail-closed protected Unicode identities and committed-state projection.


✏️ Learnings added
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T14:13:43.868Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `preserve_quoted_special_role_specifications()` must preserve immediately `U&`-prefixed lowercase quoted identifiers such as `U&"current_user"`, `U&"current_role"`, and `U&"session_user"`. These are named Unicode-escaped role identities, not executor-relative pseudo-targets. The helper must continue to rewrite ordinary lowercase quoted special-role spellings before lifecycle normalization so they cannot alias unquoted `CURRENT_USER`, `CURRENT_ROLE`, or `SESSION_USER`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T10:14:43.914Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, `update_targets_unsafe_replication_role_via_pg_settings()` must treat a structurally projected Unicode-escaped `U&"pg_catalog"` schema as canonical when it qualifies `pg_settings`. It must fail closed for `INVALID_QUOTED_IDENTIFIER` projections and for any remaining structural `U&` marker after canonical `pg_settings` identity is established. This prevents Unicode-escaped relation, alias, or assignment-column identities from bypassing the committed replica execution-policy check until the shared lexer owns Unicode escape decoding.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T07:10:14.777Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_validation.rs`, a committed `SET LOCAL session_replication_role = replica` is intentionally rejected as a fail-closed execution-policy violation. Although PostgreSQL resets `SET LOCAL` at transaction end, a committed transaction can execute protected DML while ordinary TEPP enforcement triggers are suppressed. Rolled-back replica-mode statements remain discarded by the shared transaction projection.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T06:10:55.597Z
Learning: For TEPP migration validation, `SET LOCAL session_replication_role = replica` is transaction-scoped and does not persist after `COMMIT` or `ROLLBACK`. In `crates/persistence_postgres/src/migration_validation.rs`, runtime-safety validation must reject committed session-level `SET` or `SET SESSION` changes to `replica`, but must not reject a completed `SET LOCAL` change solely as a durable final-state hazard.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review current exact head 2279ede859f304b70d43aded832a7dfca84b0de6, scoped to #590. Public RED lineage is 49e40000a7f0c85747d9f94593b865dfbb2fb975 → supplemental qualified-target RED 156b873cef8bf3fe91ee729c50f82da817ed684c → causal repair 2279ede859f304b70d43aded832a7dfca84b0de6. Focus on migration_rls_table_state::direct_table_target(): PostgreSQL schema qualification with whitespace around . must not collapse sibling relations into the schema token and overwrite final RLS state. Verify compact, public . tenant_record, public .tenant_record, unqualified controls, and rollback semantics. This remains a Draft source review; skipped/no hosted CI is not promotion evidence.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='2279ede859f304b70d43aded832a7dfca84b0de6'
red1='49e40000a7f0c85747d9f94593b865dfbb2fb975'
red2='156b873cef8bf3fe91ee729c50f82da817ed684c'
core='crates/persistence_postgres/src/migration_core.rs'
state='crates/persistence_postgres/src/migration_rls_table_state.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'
lexical='crates/persistence_postgres/src/migration_validation_impl.rs'

printf '%s\n' '== exact revision and lineage =='
printf 'checkout='; git rev-parse HEAD
git diff --quiet "$head" HEAD && echo 'checkout-matches-requested-head'
for revision in "$red1" "$red2" "$head"; do
  git show --no-patch --format='revision=%H%nparents=%P%nsubject=%s' "$revision"
  git merge-base --is-ancestor "$revision" "$head" && echo 'is-ancestor=yes' || echo 'is-ancestor=no'
done
git log --oneline "${red1}^..${head}"
git diff --check "${head}^" "$head"

printf '%s\n' '== locate source and test files =='
fd -a 'migration.*(rls|table|state).*\.rs|migration_transaction_projection\.rs|migration_validation_impl\.rs' crates/persistence_postgres || true
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests |
  rg -i 'migration.*(rls|table.*state|transaction).*\.rs$' || true

printf '%s\n' '== scoped diff =='
git diff --name-status "${red1}^" "$head" -- crates/persistence_postgres/src crates/persistence_postgres/tests
git diff --unified=100 "$red2" "$head" -- crates/persistence_postgres/src crates/persistence_postgres/tests

printf '%s\n' '== source outlines =='
for file in "$core" "$state" "$projection" "$lexical"; do
  if git cat-file -e "${head}:${file}" 2>/dev/null; then
    echo "--- $file ---"
    ast-grep outline "$file" --items all --type function,struct,enum,impl || true
  fi
done

printf '%s\n' '== direct_table_target and state callers =='
git grep -n -C 60 -E \
  'direct_table_target|migration_rls_table_state|ENABLE ROW LEVEL SECURITY|DISABLE ROW LEVEL SECURITY|FORCE ROW LEVEL SECURITY|NO FORCE ROW LEVEL SECURITY|project_committed_sql|ROLLBACK|ABORT' \
  "$head" -- crates/persistence_postgres/src crates/persistence_postgres/tests

printf '%s\n' '== focused source snapshots =='
for file in "$core" "$state" "$projection" "$lexical"; do
  if git cat-file -e "${head}:${file}" 2>/dev/null; then
    echo "--- $file ---"
    git show "${head}:${file}" | nl -ba | sed -n '1,1200p'
  fi
done

printf '%s\n' '== relevant public tests =='
git ls-tree -r --name-only "$head" crates/persistence_postgres/tests |
  rg -i 'migration.*(rls|table.*state|transaction).*\.rs$' |
  while IFS= read -r file; do
    echo "--- $file ---"
    git show "${head}:${file}" | nl -ba
  done

printf '%s\n' '== structural source checks =='
python3 - <<'PY'
import subprocess
head='2279ede859f304b70d43aded832a7dfca84b0de6'
for path in (
    'crates/persistence_postgres/src/migration_core.rs',
    'crates/persistence_postgres/src/migration_rls_table_state.rs',
    'crates/persistence_postgres/src/migration_transaction_projection.rs',
    'crates/persistence_postgres/src/migration_validation_impl.rs',
):
    try:
        source = subprocess.check_output(['git', 'show', f'{head}:{path}'], text=True)
    except subprocess.CalledProcessError:
        continue
    print(f'{path}: brace_delta={source.count("{") - source.count("}")}')
PY

Length of output: 116120


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='2279ede859f304b70d43aded832a7dfca84b0de6'
red1='49e40000a7f0c85747d9f94593b865dfbb2fb975'
red2='156b873cef8bf3fe91ee729c50f82da817ed684c'
state='crates/persistence_postgres/src/migration_rls_table_state.rs'
test='crates/persistence_postgres/tests/migration_rls_table_final_state_contract.rs'
projection='crates/persistence_postgres/src/migration_transaction_projection.rs'

printf 'checkout='; git rev-parse HEAD
printf 'red1='; git rev-parse "$red1"
printf 'red2='; git rev-parse "$red2"
git merge-base --is-ancestor "$red1" "$head" && echo red1-is-ancestor
git merge-base --is-ancestor "$red2" "$head" && echo red2-is-ancestor
git show --no-patch --format='head_parents=%P%nhead_subject=%s' "$head"
git log --oneline "${red1}^..${head}"

echo '== exact repair diff =='
git diff --unified=100 "$red2" "$head" -- "$state" "$test"

git show "${head}:${state}" > /tmp/migration_rls_table_state_590.rs
echo '== direct_table_target definitions and callers =='
rg -n -C 80 'fn direct_table_target\b|direct_table_target\(|struct .*Rls|enum .*Rls|ALTER|ENABLE|DISABLE|FORCE|NO|ROLLBACK|ABORT' /tmp/migration_rls_table_state_590.rs

echo '== full compact state module =='
nl -ba /tmp/migration_rls_table_state_590.rs | sed -n '1,700p'

echo '== scoped contract =='
git show "${head}:${test}" | nl -ba

echo '== committed projection interface =='
git show "${head}:${projection}" | nl -ba | sed -n '1,260p'

echo '== exact surrounding caller integration =='
git grep -n -C 35 -E 'migration_rls_table_state|RlsTableState|direct_table_target|project_committed_sql' \
  "$head" -- crates/persistence_postgres/src/migration_core.rs crates/persistence_postgres/src/migration_validation.rs crates/persistence_postgres/src/migration_rls_table_state.rs

echo '== whitespace =='
git diff --check "${head}^" "$head"

Length of output: 106562


@seonghobae I reviewed #590 at exact head 2279ede859f304b70d43aded832a7dfca84b0de6.

The checkout matches the requested head. Both public RED commits are ancestors. The current repair is the direct child of supplemental RED 156b873cef8bf3fe91ee729c50f82da817ed684c.

I found no source-level bypass or false positive in the requested scope.

migration_rls_table_state::direct_table_target() now rejects both qualification forms:

ALTER TABLE public.tenant_record ...
ALTER TABLE public . tenant_record ...
ALTER TABLE public .tenant_record ...

The helper rejects a period inside the target token. It also rejects a period at the start of the following token. Therefore, it cannot use public as the state-map key for two different qualified relations.

The RLS state fold returns false for an RLS action with an unsupported qualified target. validate_migration_catalog() then returns MigrationContractError::MissingRlsEnable. A sibling relation cannot restore the state of a disabled target.

Unqualified targets still enter the state map by their table name. The existing enable, force, disable, and no-force ordering remains unchanged.

migration_rls_table_final_state_contract.rs covers both whitespace forms:

public . tenant_record
public .tenant_record

The contract also verifies that a rolled-back qualified disable does not affect final state. validate_migration_catalog() applies project_committed_sql() before the RLS table-state fold. The transaction projection discards statements in a transaction that ends with ROLLBACK or ABORT.

The focused Rust change has balanced delimiters. git diff --check reported no whitespace errors. I did not build or run the code.

I used the existing PR guidance that Draft-skipped CI is not execution or promotion evidence. This result is exact-head source-review evidence only.


🧠 Learnings used
Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-18T05:08:21.910Z
Learning: For TEPP migration validation in `crates/persistence_postgres/src/migration_core.rs`, protected append-only and retention guard routines have canonical `public` schema identity. The guard-routine final-state matcher must accept only the unqualified canonical routine name or an explicit `public.` qualification, including whitespace around the qualification dot. A same-named routine in another schema, such as `audit_support.reject_held_evidence_deletion`, is unrelated and must not cause a final-state mutation error.

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/TEPP/pull/521

Timestamp: 2026-09-17T03:01:59.380Z
Learning: For PR `#521`, Draft or lifecycle-skipped CI is not GREEN and is not promotion evidence. Migration validator review conclusions require exact-head source or executed-gate evidence.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant