feat: apply desired schemas through migrate

[aparajon]

Why

The desired-state library and atomic row level security (RLS) executor already exist, but CLI users cannot apply those declarations. This connects them to migrate --desired schema.sql.

What

• Add --desired and its target --schema, mutually exclusive with --alter; reject force and blocking overrides for desired files
• Route ordinary declarations through RunDesired and explicit RLS declarations through the atomic executor
• Add command tests, a built-binary demo, and a Supabase walkthrough; update the capability matrix

How

Ordinary changes retain the existing plan and per-statement verdicts. RLS changes return one verdict with only committed SQL. The executor derives the change under its lock and verifies convergence before commit; mixed table/RLS changes remain refused.

--dry-run never applies changes. If any table-only step is destructive, the entire preview is refused, all execution advice is withdrawn, and the final preview is fingerprinted. The aggregate refusal takes the same precedence as apply, even if routing already refused a step. RLS previews retain the existing review-only response and exit 2 when definitions differ, even when an explicit apply can execute the change. No approval fingerprints or RLS-specific flags are introduced.

Risk

This exposes existing executors through a new CLI input mode. Executor changes only preserve typed privilege causes; execution and locking behavior stay the same. RLS declarations own the complete policy set and can deliberately widen access. Failures preserve executor codes, including an unknown commit outcome. Registry-backed refusals distinguish missing privileges from unsupported changes. A missing RLS table is environmental in both preview and apply. Hosted Supabase support is not established by these tests.

Testing

No manual testing. Automated coverage includes PostgreSQL command integration tests and the built-binary smoke tour in CI.

Bigger picture

Follows #123 and #125: atomic RLS execution and local Supabase API validation now have a CLI workflow. Ordinary desired changes keep committed-prefix semantics; RLS changes commit together. Hosted validation remains a follow-up.

Generated with Codex (GPT-6)

[Copilot]

Copilot review overview

🟡 Changes recommended

Destructive previews emit inconsistent plan metadata, and RLS refusal output conflicts with the established verdict contract.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 · 2

Open (5)

• Add tests for migrate and schema command validation · New
• Use refusal helper and recompute fingerprint after mutations · New
• Avoid assigning failure codes to refused outcomes · New
• Align exit-code documentation with semantic error handling · New
• Track required fixture image updates in a separate PR · New

What changed in this PR

Adds CLI support for applying desired-state schemas, including atomic RLS changes, through migrate --desired.

Changes:

• Adds desired-schema CLI validation, planning, execution, and result rendering.
• Routes explicit RLS declarations through the atomic executor.
• Adds integration coverage, demo fixtures, and capability documentation.

File	Description
README.md	Documents CLI-based RLS execution.
pkg/​capabilities/​capabilities.yaml	Marks declarative RLS migration as supported.
internal/​cli/​migrate.go	Dispatches desired-state requests.
internal/​cli/​migrate_row_security.go	Connects RLS declarations to the atomic executor.
internal/​cli/​migrate_row_security_integration_test.go	Tests RLS failures and policy removal.
internal/​cli/​migrate_desired.go	Implements desired-state migration and dry runs.
internal/​cli/​migrate_desired_test.go	Tests validation and RLS verdict handling.
internal/​cli/​migrate_desired_integration_test.go	Tests table and RLS convergence paths.
internal/​cli/​cli.go	Adds --desired and --schema flags.
docs/​supabase.md	Adds the Supabase RLS application workflow.
docs/​limitations.md	Updates RLS limitations.
docs/​declarative-row-security.md	Documents execution behavior and output.
docs/​capabilities.md	Updates the rendered capability matrix.
docs/​atomic-row-security.md	Documents CLI availability.
demo/​tour.sh	Adds built-binary RLS smoke coverage.
demo/​seed.sql	Adds the demo RLS table.
demo/​desired-rls.sql	Defines the demo’s desired RLS state.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for pg-sprite/pull/126, f38f1d3.

Verdict: 5 findings — 1 blocking (RLS privilege refusal misclassified), 4 non-blocking (refusal registry, dry-run class, stale docs).

Blocking

RLS privilege refusals are reported as unsupported-statement / capability-boundary instead of insufficient-privileges / environmental. internal/cli/migrate_row_security.go:51 maps every ErrRowSecurityRefused to v.WithRefusal(verdict.CapabilityBoundary(verdict.ReasonUnsupportedStatement)). checkRowSecurityPrivileges (non-owner role, missing database CREATE) and rowSecurityError (SQLSTATE 42501) wrap that same sentinel, so a role with CREATE that does not own public.documents gets {refused, unsupported-statement, capability-boundary} and SchemaBot routes it as "wait or escalate" when the fix is a grant. The greenfield path returns insufficientPrivilegesRefusal() for the same condition, and no CLI test covers the privilege branch.

Non-blocking

The two new RLS refusal sites pick their class in internal/cli, outside the refusal registry. internal/cli/migrate_row_security.go:54 mints CapabilityBoundary and Environmental itself, but pkg/verdict/verdict.go:197 says the registries in pkg/plan and pkg/migrate mint them and "a refusal site never names a class on its own". TestRefusalRegistryIsComplete never reaches these sites and docs/refusal-classes.md does not list them, so changing either class, or adding a new ErrRowSecurityRefused cause, fails no test.

migrate --desired --dry-run on an RLS file emits a refused verdict with no class. internal/cli/diff_row_security.go:43 builds verdict.Verdict{Outcome: verdict.OutcomeRefused, Reason: verdict.ReasonUnsupportedStatement, ...} directly instead of calling WithRefusal, which breaks RF-7 ("Every refusal carries exactly one non-zero class"). The line predates this PR, but this PR is what makes the migrate command reach it for a policy change.

docs/capabilities.md has two hand-written statements that are now false. docs/capabilities.md:33 still says the RLS executor is "a Go API only, with no migrate or diff execution", and lines 55-56 say "Both doors carry the same disposition on every row today". declarative-row-security-library is now migrate: supported / diff: refused, which is the only row where the doors differ, so a .front_doors.diff query following the recipe would miss it.

The docs update missed the non-RLS text: README and three other docs still say desired-state execution has no CLI verb. README.md:82 ("Desired-state execution has no CLI verb yet"), README.md:44, docs/limitations.md:39 and :44, and docs/optimistic-attempt.md:157 all say it is library-only. migrate --desired now runs migrate.RunDesired for ordinary table files, including greenfield CREATE TABLE (see TestMigrateDesiredCreatesMissingTable).

The one thing that could have broken, verified

migrate --desired reporting RLS SQL as executed when the commit did not land. rowSecurityVerdict clears ExecutedSQL on every error. RowSecurityOutcomeUnknownError keeps its own code because rowSecurityError returns it unchanged. So an unknown-commit outcome is never shown as executed, and it is never folded into a refusal.

Verified correct

• Exit codes: errors.Join(emitErr, runErr) gives exit 2 on refusal (via ErrRefused, no FatalIfErrorf), and exit 1 on failure or writer error.
• The plain runDesired path handles all three RunDesired result/error shapes (zero outcome, failed, refused) correctly.
• validateInput requires exactly one of --alter / --desired, runs in both Kong Validate and run(), and rejects --force / --accept-blocking with --desired.
• --schema requires --desired uses flagSupplied, so the public default does not trip it, and no schema flag is duplicated.
• RefuseDestructive touches only destructive statements, keeps the earlier reason/class/cause, and leaves the fingerprint unchanged for non-destructive plans.
• The plain dry-run's DiffCmd leaves out Schema without harm (writeDiffReport never reads it). The RLS dry-run passes it through.
• Executor missing table (ErrNoRows becomes executor.ErrTableNotFound) matches the errors.Is check in rowSecurityVerdict, and the integration test confirms the refused outcome.

This review was generated by Claude Code (claude-opus-5).

[morgo]

🤖 Automated adversarial review, posted on Morgan Tocker's behalf.

The wiring is careful and most of what I went looking for is already right. Specifically checked:

• Routing on parse failure is sound, not fragile as it first looks. runDesired only reaches the RLS executor when ParseDesired errors, which reads like it could misroute — but it cannot. ParseDesired's admitDesiredStatement admits CreateStmt and IndexStmt only, so any RLS declaration makes it fail; a file carrying RLS can never silently take the ordinary path. And a genuine syntax error does not get swallowed, because HasRowSecurityDeclaration re-parses with pgquery.Parse and returns detectErr, so the original ParseDesired error is what surfaces. Table-level diagnoses survive the reroute too: ParseDesiredWithRowSecurity re-runs ParseDesired over the CreateStmt/IndexStmt subset, so a REFERENCES clause still reports ErrForeignKey rather than an RLS-flavoured error.
• Re-fingerprinting in RefuseDestructive is correct and matches the convention. The other two mutators (RefuseUnsupportedPartitionedParent, RefuseUnsupportedCreateShape) do not rehash, which initially looked inconsistent — but they run inside diffplan.Plan, which sets report.Fingerprint as its last step. RefuseDestructive runs after Plan returns, so it must rehash, and does. Backend, Disposition and ExecSQL are all fingerprint inputs, so skipping it would have left a stale identity.
• The load-bearing claim in the dry-run comment holds. "which RunDesired refuses before execution" is true: admitPlan walks report.Statements and refuses on the first ps.Destructive. migrate --desired cannot execute a destructive step.
• Exit codes are preserved rather than reinvented — writeDiffReport returns verdict.ErrRefused on a refused plan, so --desired --dry-run gates in CI the same way --alter --dry-run does.

One finding, and three notes.

The destructive preview disagrees with the admission it says it mirrors

// Preview uses desired-state admission: a routed native step can still
// discard live structure, which RunDesired refuses before execution.
plan.RefuseDestructive(&report)

RefuseDestructive refuses per statement:

if report.Statements[i].Destructive {
    return verdict.ByDesign(verdict.ReasonDestructiveChange), ""
}
return verdict.Refusal{}, ""

admitPlan refuses the whole plan — it returns at the first destructive statement and nothing runs. Its own skippedRestDetail says so out loud: "admission is all-or-nothing, so the plan's other statement, even if non-destructive, was not run".

So the preview and the execution describe different outcomes for the same file.

Failure scenario. A desired file drops a column and adds an index. migrate --desired --dry-run --json renders the drop with disposition: refuse and the index with disposition: execute and its exec_sql intact. The top-level disposition is refuse and the exit code is 2, so a CI gate is fine — but an operator reading the statement list concludes the index build will proceed and the drop will be skipped. They run without --dry-run and get a whole-plan refusal with zero statements executed and the index not created.

The direction is the safe one (the preview over-promises; execution under-delivers), which is why I am not calling this data loss. But it is still a preview that discloses exec_sql for a statement that cannot run, and this codebase's whole refuseStatements mechanism — "withdraws every piece of execution advice" — exists to prevent exactly that.

This is pinned by a test rather than incidental, so it needs a deliberate answer rather than a one-line patch. TestRefuseDestructiveWithdrawsExecutionAndRehashes asserts assert.Equal(t, untouched, report.Statements[1]), i.e. that the non-destructive sibling keeps its execute disposition. And the integration coverage does not reach the case: TestMigrateDesiredTableRefusesDestructiveChange asserts require.Len(t, preview.Statements, 1), so no test compares a mixed plan's preview against what executing it actually does.

Matching admitPlan means refusing every statement once any is destructive — a first pass to detect, then a selector that returns the refusal for all indices — and the sibling would carry the by-design destructive reason as the explanation for why it, too, will not run. If per-statement marking is instead the intent, then the comment should not say the preview uses desired-state admission, and the report needs to say somewhere that admission is all-or-nothing, or the operator has no way to learn it before running.

Notes

An RLS file without its CREATE TABLE reports "empty desired schema". ParseDesiredWithRowSecurity builds tableSQL from CreateStmt/IndexStmt nodes only, then calls ParseDesired on it; a policies-and-settings-only file yields an empty string and ErrEmptyDesired. The docs say to apply the file pull produced, which always carries the table, so this is not on the documented path — but "apply an RLS-only declaration to an existing supported table" reads like an invitation to hand-write exactly that file, and empty desired schema is a misleading answer to it. A dedicated error naming the missing CREATE TABLE would cost one branch.

runDesiredRowSecurity parses, then discards the result on the dry-run path. ParseDesiredWithRowSecurity runs before the c.DryRun check, and runRowSecurityDiff(ctx, out, sql) re-parses the same string. It works as an admission check, but a reader will wonder whether desired was meant to be threaded through. A comment saying the first parse is there to fail admission before any preview, or moving it under the non-dry-run branch and letting the diff path report its own parse error, would settle it.

Inconsistent DiffCmd construction. The RLS path passes Schema: c.Schema; the ordinary dry-run path omits it. writeDiffReport does not read Schema, so nothing is broken today — but the two lines sit ten lines apart and differ, which invites the reader to hunt for the significance. Worth making them identical.

[aparajon]

🤖 Addressed all five findings from this review in d099e71.

• Owner/database CREATE failures and SQLSTATE 42501 now retain a typed privilege cause. The migrate registry maps it to insufficient-privileges / environmental, before the general RLS admission sentinel.
• Execution and preview refusals now use the existing migrate/plan registries. RLS previews carry class: capability-boundary; registry tests and refusal documentation cover the mappings.
• Updated the handwritten capability guidance, README, limitations, and execution walkthrough to describe migrate --desired accurately.

Real PostgreSQL CLI tests cover a non-owner with database CREATE and an owner without it, asserting the refusal classification and unchanged catalog. The SQLSTATE cause, preview class, and registry mappings also have regression coverage. Focused integration tests, unit/safety tests, race checks, lint, and the built-binary demo passed. CI is running on the new commit.

Generated with Codex (GPT-6)

[aparajon]

🤖 Addressed the destructive-preview finding and all three notes in d099e71.

RefuseDestructive now refuses the entire preview when any step is destructive, withdraws execution advice from every step, and recomputes the fingerprint. The updated PostgreSQL CLI test drops a column and adds an index in the same desired file: both preview steps are refused, apply executes nothing, and the catalog stays unchanged. A separate unit test keeps ordinary non-destructive plans unchanged.

A settings/policies-only file now names the missing CREATE TABLE definition. Dry-run delegates parsing to the diff path once, and both DiffCmd constructions carry Schema consistently.

Focused integration tests, unit/safety tests, race checks, lint, and the built-binary demo passed. CI is running on the new commit.

Generated with Codex (GPT-6)

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for pg-sprite/pull/126, d099e71.

Re-review of the delta only: f38f1d30..d099e71d, since the last delivered review at f38f1d30.

Verdict: 4 findings — 3 non-blocking (preview/apply refusal drift, GRANT contract), 1 suggestion.

Non-blocking

1. When a destructive plan is already route-refused, the desired preview's aggregate reason and class can differ from what the run reports. pkg/plan/plan.go#L257 only stamps destructive-change on the report when it is not already refused (if report.Disposition != router.DispositionRefuse {), and pkg/plan/desired.go#L19 routes the preview through that path. Take a desired file that drops a column and adds an EXCLUDE constraint, which the planner refuses. --dry-run --json shows aggregate refuse with reason "" and class "", but the run's admitPlan checks destructive statements first and refuses with destructive-change / by-design (pkg/migrate/desired.go#L375). The guard predates this PR, but the whole-plan alignment this PR set out to make is only partly done, and no test covers this case.

2. The RLS preview labels a missing target table capability-boundary, but applying the same file labels it environmental. internal/cli/diff_row_security.go#L44 applies RowSecurityReviewRefusal() to every ErrUnsupportedChange, including the ErrTableNotFound that pkg/diffplan/row_security.go#L32 wraps. Apply reaches rowSecurityMissingTableRefusal() (environmental), which is also what docs/refusal-classes.md#L251 documents. So automation that branches on class treats the preview as "wait for the engine" and the apply as "fix the environment". The helper's "keeps policy deltas review-only" comment also does not cover this case.

3. RLS privilege refusals now carry insufficient-privileges, but their detail never names a GRANT. The documented contract says that reason's detail names the exact missing GRANT (docs/cli-output-examples.md#L116). The non-owner detail from pkg/executor/row_security_owner.go#L32 names no GRANT, and ownership cannot be granted anyway. A 42501 only passes through raw PostgreSQL text. Either document an RLS carve-out or put an actionable remedy in the detail.

General suggestions

4. The new privilege-refusal test tells its two cases apart only by matching words in the human-readable Detail. In internal/cli/migrate_row_security_integration_test.go#L135, non-owner and missing database CREATE share the same sentinels, reason, and class, so assert.Contains(t, v.Detail, "requires CREATE on the database") is the only thing separating them. AGENTS.md says to make such a difference machine-readable (a typed field or reason) and assert that instead. pkg/statement/desired_rls_test.go#L119 (assert.Contains(t, err.Error(), "CREATE TABLE")) has the same weakness.

The one thing that could have broken, verified

Sentinel ordering in RowSecurityRefusal: privilege failures wrap both ErrRowSecurityPrivileges and ErrRowSecurityRefused, so checking them in the wrong order would misclassify every privilege failure as a generic RLS refusal. The code checks ErrRowSecurityPrivileges first. Both privilege sites and the 42501 path wrap both sentinels, so they all classify as insufficient-privileges / environmental.

Verified correct

• A 42501 from scratch inspection (wrapped only in ErrRowSecurityRefused) still reaches rowSecurityError's 42501 branch and picks up ErrRowSecurityPrivileges.
• On apply, a missing table in checkRowSecurityPrivileges wraps only ErrTableNotFound and classifies as environmental.
• Moving ParseDesiredWithRowSecurity after the DryRun branch loses nothing, because runRowSecurityDiff parses the same SQL first.
• WithRefusal sets Outcome=refused, so the RLS refusal still emits a refused verdict and now carries the .class the demo asserts.
• RefuseDestructive leaves a plan with no destructive statements untouched, fingerprint included.
• A destructive plan withdraws backend, exec_sql, and execution from every statement, which matches admitPlan's all-or-nothing rule.
• refuseStatements keeps an already-refused statement's own reason and class.
• The Schema flag now reaches the DiffCmd for the non-RLS desired preview (it was dropped before).

This review was generated by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Approved on Kiran's (@kmuddukrishna) behalf by the scheduled review agent — no blocking findings at d099e71. See the review comment above; non-blocking findings and suggestions, if any, are not merge gates.

[aparajon]

🤖 Addressed all four points from the re-review in c4e94a7.

• Destructive admission now takes precedence in the aggregate preview even when routing already refused a step. Per-statement refusals remain specific. A PostgreSQL test combines a dropped column with an EXCLUDE constraint and checks matching preview/apply reason and class, zero execution, and an unchanged catalog.
• Missing RLS targets now use the same registry refusal in preview and apply: unsupported-statement / environmental. The CLI regression covers both paths.
• Owner and database CREATE refusals now include an actionable remedy. The output contract distinguishes these RLS admission diagnostics from preflight's exact GRANT advice and from raw PostgreSQL permission diagnostics.
• Added typed causes for missing ownership, database CREATE, and the required table definition. Tests assert those causes instead of matching human-readable prose; existing enclosing sentinels remain intact.

Focused PostgreSQL tests, unit/safety checks, race checks, lint, and the built-binary smoke tour passed. The new commit is running through CI; the preceding commit was all green.

Generated with Codex (GPT-6)