fix(sql): recover CREATE POLICY via regex fallback; add pg_policy introspection (#3401) - #3403
fix(sql): recover CREATE POLICY via regex fallback; add pg_policy introspection (#3401)#3403AromalBiju1 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new policy extraction/introspection paths contain correctness issues (duplicate regex definition, policy→function reference resolution, and PUBLIC role DDL reconstruction) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves graphify’s Postgres SQL coverage by ensuring CREATE POLICY statements become first-class graph nodes both in static SQL extraction (tree-sitter error fallback) and in live --postgres introspection, so RLS policies are no longer silently omitted from generated graphs.
Changes:
- Add a whole-file regex recovery path in
graphify/extractors/sql.pyto extractCREATE POLICYnodes and edges when tree-sitter cannot parse them. - Extend
graphify/pg_introspect.pyto querypg_catalog.pg_policyand reconstruct policy DDL for the shared SQL extraction pipeline. - Add fixtures/tests for policy extraction; additionally includes Cursor skill packaging + install/uninstall behavior changes.
File summaries
| File | Description |
|---|---|
| uv.lock | Updates lockfile content (broad dependency marker churn). |
| pyproject.toml | Adds skill-cursor.md to packaged data files. |
| graphify/skill-cursor.md | New Cursor /graphify skill content (command + references guidance). |
| graphify/install.py | Adds Cursor platform skill installation + uninstallation behavior. |
| graphify/extractors/sql.py | Adds regex fallback recovery for CREATE POLICY extraction. |
| graphify/pg_introspect.py | Adds pg_policy query + reconstructed CREATE POLICY DDL generation. |
| tests/fixtures/policies.sql | Fixture SQL containing table, function, and two policies. |
| tests/test_create_policy_snippet.py | New end-to-end test asserting policy nodes + edges exist. |
| tests/test_install.py | Adds Cursor install/uninstall tests for skill command + references sidecar. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds Cursor to the platform config so project install now writes both the always-on .cursor/rules/graphify.mdc rule and a new .cursor/commands/graphify.md skill file (via _cursor_install), giving /graphify a defined procedure on Cursor; _cursor_uninstall removes both and reports "nothing to remove" when neither existed. Extracts PostgreSQL CREATE POLICY statements two ways: pg_introspect now queries pg_policy and reconstructs full policy DDL (command, permissive/restrictive, roles, USING/WITH CHECK), and the SQL extractor recovers policies via a whole-file _POLICY_RECOVERY_RX fallback (the grammar has no policy rule), emitting a policy node with an applies_to edge to its table and references edges to functions called in the USING/CHECK expressions. Note: _POLICY_RECOVERY_RX is defined twice in a row, the second assignment shadowing the first.
Worth a look
- User question is interpolated into a shell command —
graphify/skill-cursor.md· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- All-cached semantic path skips creation of .graphify_semantic.json —
graphify/skill-cursor.md· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Policy recovery treats SQL predicates like IN as function references —
graphify/extractors/sql.py:111· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Policy references self-referential function edges to its own table via _ref_stub —
graphify/extractors/sql.py:745· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Cursor rule now removed even when it wasn't graphify-owned —
graphify/install.py:1178· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 613 functions depend on the 350 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
extract_sql()— 22 callers, 9 callees - new:
codebuddy_install()— 20 callers, 5 callees - new:
claude_install()— 19 callers, 4 callees - new:
gemini_install()— 10 callers, 7 callees - new:
claude_uninstall()— 17 callers, 4 callees - new:
_project_uninstall()— 5 callers, 13 callees - new:
_copy_skill_file()— 13 callers, 5 callees - …and 15 more — each is listed as a finding
Verification — 613 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 541 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_cursor\_install.
The verifier did not have enough to check \_cursor\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 29 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_cursor\_uninstall (not a proof).
The verifier ran both versions of \_cursor\_uninstall on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify introspect\_postgres.
The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)
· 23 more finding(s) on lines outside this diff (see the check run).
c2b1307 to
770ab8d
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds first-class RLS policy support end to end: _POLICY_RECOVERY_RX in the SQL extractor recovers CREATE POLICY statements (which tree-sitter has no grammar for) via whole-file fallback, emitting a policy node with an applies_to edge to its table and references edges to functions called in USING/WITH CHECK clauses, while introspect_postgres now reads pg_policy and reconstructs equivalent CREATE POLICY DDL from the live database. Adds a /graphify skill file for Cursor so _cursor_install writes .cursor/commands/graphify.md alongside the always-on .mdc rule (giving the command an actual procedure instead of just a nudge), and _cursor_uninstall now removes both, printing "nothing to remove" when neither exists.
Worth a look
- User question is interpolated into a shell command using double quotes —
graphify/skill-cursor.md· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- INPUT_PATH is interpolated into Python source —
graphify/skill-cursor.md:114· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Policy recovery drops USING bodies with nested parentheses —
graphify/extractors/sql.py:87· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Postgres introspection now requires newer pg_policy catalog columns —
graphify/pg_introspect.py:106· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Shell command injection via INPUT_PATH substitution —
graphify/skill-cursor.md· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 613 functions depend on the 350 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
extract_sql()— 22 callers, 9 callees - new:
codebuddy_install()— 20 callers, 5 callees - new:
claude_install()— 19 callers, 4 callees - new:
gemini_install()— 10 callers, 7 callees - new:
claude_uninstall()— 17 callers, 4 callees - new:
_project_uninstall()— 5 callers, 13 callees - new:
_copy_skill_file()— 13 callers, 5 callees - …and 15 more — each is listed as a finding
Verification — 613 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 541 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_cursor\_install.
The verifier did not have enough to check \_cursor\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 29 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_cursor\_uninstall (not a proof).
The verifier ran both versions of \_cursor\_uninstall on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify introspect\_postgres.
The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)
· 23 more finding(s) on lines outside this diff (see the check run).
770ab8d to
bba8762
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds CREATE POLICY recovery to the SQL extractor via _POLICY_RECOVERY_RX, a whole-file regex fallback (the grammar has no policy rule, so there's no walk-time node to dispatch on) that emits a policy node with an applies_to edge to its table and references edges to any functions called in the USING/WITH CHECK expressions. Each TO/USING/WITH CHECK clause is its own optional group so bare or USING-only policies still match. Extends introspect_postgres to query pg_policy and re-emit each policy as CREATE POLICY ... AS PERMISSIVE/RESTRICTIVE FOR <cmd> TO <roles> DDL, mapping polcmd codes to command names and defaulting roles to public.
Worth a look
- Function definitions registered in table_nids can shadow table references, misresolving FK REFERENCES to a function node —
graphify/extractors/sql.py:718· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Postgres policy introspection drops PUBLIC from mixed role policies —
graphify/pg_introspect.py:108· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 242 functions depend on the 30 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
extract_sql()— 22 callers, 9 callees - new:
walk()— 1 callers, 8 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 242 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 78 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify introspect\_postgres.
The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)
· 4 more finding(s) on lines outside this diff (see the check run).
…ed-role policy DDL
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds CREATE POLICY recovery to the SQL extractor via a whole-file _POLICY_RECOVERY_RX fallback (the grammar has no rule for policies), emitting a node per policy with an applies_to edge to its table and references edges to any functions called in the USING/WITH CHECK bodies. Extends introspect_postgres to query pg_policy and reconstruct equivalent CREATE POLICY DDL, mapping polcmd codes to command names and rendering roles as PUBLIC or quoted identifiers. Each clause (AS, FOR, TO, USING, WITH CHECK) is independently optional so USING-only policies aren't dropped.
Worth a look
- Policy recovery drops USING/WITH CHECK bodies with nested parentheses —
graphify/extractors/sql.py:96· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- applies_to target label mismatch: table node created as bare name but edge points to qualified stub —
graphify/extractors/sql.py:727· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Policy function scanner fabricates references for SQL keywords —
graphify/extractors/sql.py:734· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 242 functions depend on the 30 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
extract_sql()— 22 callers, 9 callees - new:
walk()— 1 callers, 8 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 242 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 78 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify introspect\_postgres.
The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)
· 4 more finding(s) on lines outside this diff (see the check run).
…ed-role policy DDL; handle nested parens and SQL keywords in policy body scan
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds CREATE POLICY extraction to the SQL extractor via a whole-file regex fallback (the grammar has no rule for it, so there's no walk node to dispatch on), emitting each policy as a node with an applies_to edge to its table and references edges to functions called in the USING/WITH CHECK bodies. Locates the predicate bodies with a manual balanced-paren scan (_match_balanced_parens) rather than a fixed-depth pattern, and filters out SQL keywords so only real function calls become references. Teaches introspect_postgres to dump live RLS policies from pg_policy as reconstructed CREATE POLICY DDL, resolving commands, roles, and USING/WITH CHECK expressions.
No blocking issues surfaced. 11 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 243 functions depend on the 31 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
extract_sql()— 22 callers, 10 callees - new:
walk()— 1 callers, 8 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 243 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 79 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify introspect\_postgres.
The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)
· 4 more finding(s) on lines outside this diff (see the check run).
Fixes #3401
Problem
CREATE POLICYis silently dropped from SQL graphs. tree-sitter-sql's grammarhas no rule for it — policy statements disintegrate into loose tokens plus an
ERROR node, the same dead end CREATE FUNCTION/PROCEDURE bodies hit. The
existing ERROR-fallback regex in
extractors/sql.pyonly coversFUNCTION|PROCEDURE, so policies fall through with no node and no warning.
pg_introspect.py's--postgrespath has the same hole: it never queriespg_policy.Fix
graphify/extractors/sql.py: added_POLICY_RECOVERY_RX, a regexfallback (parallel to the existing routine-recovery regex) that runs inside
the
root.has_errorpath. Captures policy name, target table(schema-qualified), permissive/restrictive, command, roles, and the
USING/WITH CHECKbodies. Each matched policy becomes a node with anapplies_toedge to its table; function calls found in the USING/CHECKexpressions become
referencesedges.graphify/pg_introspect.py: added apg_catalog.pg_policyquery(joined to
pg_class/pg_namespace, resolvingpolqual/polwithcheckvia
pg_get_expr()rather than regexing raw SQL) so live--postgresintrospection reconstructs
CREATE POLICYDDL and feeds it through thesame pipeline as static extraction.
tests/fixtures/policies.sql: repro fixture from the issue (one table,one function, two policies referencing it).
tests/test_create_policy_snippet.py: asserts both policies land asnodes, each with an
applies_toedge topublic.employeesand areferencesedge toapp.is_admin.Review fixes
Addressed all three Copilot findings from initial review:
in
table_nids, so policyreferencesedges always fell back tosourceless stub nodes instead of the real function node. Fixed by
registering the function's id in
table_nidsat creation time.references-edge loop could emit duplicate edges when the samefunction appeared in both
USINGandWITH CHECK. Fixed by deduping onnormalized function name before emitting edges.
pg_introspect.py's DDL reconstruction quoted thePUBLICpseudo-role asa literal identifier (
TO "public"), which is invalid/incorrect DDL.Fixed to emit the bare
PUBLICkeyword when the role is the default.Testing
uv sync --all-extras
uv run pytest tests/ -q -k "sql or policy" # 44 passed
uv run pytest tests/ -q # full suite; 1 known-flaky
failure in test_labeling.py
(unrelated to this PR —
passes consistently in
isolation, order-dependent
under full-suite run)
Notes
public.employees,app.is_admin) — matches how the extractor already treats otherschema-qualified identifiers.
introspect_postgres's new policy-DDLgeneration (would need a live Postgres fixture); the static-extraction path
is covered end-to-end via the shared DDL string.
upstream/v8to drop unrelated Cursorinstall commits that were pulled in accidentally from the branch's original
base.