Replace obligation model with flat policy_type, add 5 migrations, fix definition null bug

Flattens the policy model: drops policy_obligation table and merges
targets/definition/policy_type directly onto the policy row. Adds 5
migrations (019-023) to drop old tables and recreate policy/version/
assignment with the new schema. Updates proxy engine, hooks, admin
handlers, DTOs, and admin-UI to match the flat model.

Also fixes a bug in update_policy where changing to a non-expression
type (column_allow/deny/table_deny) left a stale definition in the DB;
the handler now clears definition unconditionally for types that don't
use one. Two unit tests were added to cover this.

Author: jumpbetweenrows

.claude/agents/gemini-reviewer.md

@@ -173,7 +173,7 @@ betweenrows/
     │                                 policy_handlers, audit_handlers, policy_yaml
     ├── discovery/                    DiscoveryProvider trait + Postgres impl
     ├── entity/                       SeaORM entities (proxy_user, data_source, policy,
-    │                                 policy_obligation, policy_assignment, policy_version,
+    │                                 policy_assignment, policy_version,
     │                                 query_audit_log, …)
     ├── engine/mod.rs                 EngineCache, VirtualCatalogProvider, build_arrow_schema()
     └── hooks/                        QueryHook trait, ReadOnlyHook, PolicyHook
@@ -424,8 +424,8 @@ All policy endpoints require admin (`is_admin = true`).
 | Method | Path | Description |
 |--------|------|-------------|
 | GET | `/policies` | List policies (paginated) |
-| POST | `/policies` | Create policy with obligations |
-| GET | `/policies/{id}` | Get policy + obligations + assignment count |
+| POST | `/policies` | Create policy |
+| GET | `/policies/{id}` | Get policy + assignment count |
 | PUT | `/policies/{id}` | Update policy (requires `version` for optimistic concurrency → 409 on conflict) |
 | DELETE | `/policies/{id}` | Delete policy (cascades) |
 | GET | `/policies/export` | Export all policies as YAML |
@@ -467,9 +467,8 @@ discovered_table  (id UUID v5, discovered_schema_id, table_name, table_type, is_
 discovered_column (id UUID v5, discovered_table_id, column_name, ordinal_position,
                    data_type, is_nullable, column_default, arrow_type)
 
-policy            (id UUID v7, name, description, effect, is_enabled, version, …)
+policy            (id UUID v7, name, description, policy_type, is_enabled, version, targets JSON, definition JSON, …)
 policy_version    (id UUID v7, policy_id, version, snapshot JSON, change_type, changed_by)
-policy_obligation (id UUID v7, policy_id, obligation_type, definition JSON)
 policy_assignment (id UUID v7, policy_id, data_source_id, user_id?, priority)
 query_audit_log   (id UUID v7, user_id, username, data_source_id, datasource_name,
                    original_query, rewritten_query, policies_applied JSON,

README.md

@@ -68,9 +68,10 @@ describe('createPolicy', () => {
     mockClient.post.mockResolvedValue({ data: policy })
     const payload = {
       name: 'my-policy',
-      effect: 'permit' as const,
+      policy_type: 'row_filter' as const,
       is_enabled: true,
-      obligations: [],
+      targets: [{ schemas: ['public'], tables: ['orders'] }],
+      definition: null,
     }
     const result = await createPolicy(payload)
     expect(mockClient.post).toHaveBeenCalledWith('/policies', payload)

admin-ui/src/api/policies.test.ts

@@ -101,10 +101,10 @@ describe('PolicyAssignmentPanel', () => {
   })
 
   it('populates policy dropdown from listPolicies', async () => {
-    const policy = makePolicy({ id: 'p-1', name: 'deny-cols', effect: 'deny' })
+    const policy = makePolicy({ id: 'p-1', name: 'deny-cols', policy_type: 'column_deny' })
     mockListPolicies.mockResolvedValue({ data: [policy], total: 1, page: 1, page_size: 100 })
     renderWithProviders(<PolicyAssignmentPanel datasourceId="ds-1" />, { authenticated: true })
-    await waitFor(() => expect(screen.getByText(/deny-cols.*deny/)).toBeInTheDocument())
+    await waitFor(() => expect(screen.getByText(/deny-cols.*column_deny/)).toBeInTheDocument())
   })
 
   it('populates user dropdown from listUsers', async () => {

admin-ui/src/components/PolicyAssignmentPanel.test.tsx

@@ -178,7 +178,7 @@ export function PolicyAssignmentPanel({ datasourceId }: PolicyAssignmentPanelPro
               <option value="">Select a policy…</option>
               {allPolicies.map((p) => (
                 <option key={p.id} value={p.id}>
-                  {p.name} ({p.effect})
+                  {p.name} ({p.policy_type})
                 </option>
               ))}
             </select>

admin-ui/src/components/PolicyAssignmentPanel.tsx

@@ -1,7 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { screen, fireEvent } from '@testing-library/react'
 import { renderWithProviders } from '../test/test-utils'
-import { makePolicy, makePolicyAssignment, makeObligation } from '../test/factories'
+import { makePolicy, makePolicyAssignment } from '../test/factories'
 import { PolicyCodeView } from './PolicyCodeView'
 
 beforeEach(() => {
@@ -36,18 +36,18 @@ describe('PolicyCodeView', () => {
     fireEvent.click(screen.getByText('View as code'))
     const pre = document.querySelector('pre')!
     expect(pre.textContent).toContain('name:')
-    expect(pre.textContent).toContain('effect:')
+    expect(pre.textContent).toContain('policy_type:')
   })
 
   it('shows valid JSON when JSON toggle is clicked', () => {
-    renderCodeView({ name: 'test-policy', effect: 'permit' })
+    renderCodeView({ name: 'test-policy', policy_type: 'row_filter' })
     fireEvent.click(screen.getByText('View as code'))
     fireEvent.click(screen.getByText('JSON'))
     const pre = document.querySelector('pre')!
     expect(() => JSON.parse(pre.textContent!)).not.toThrow()
     const parsed = JSON.parse(pre.textContent!)
     expect(parsed.name).toBe('test-policy')
-    expect(parsed.effect).toBe('permit')
+    expect(parsed.policy_type).toBe('row_filter')
   })
 
   it('copy button calls navigator.clipboard.writeText', () => {
@@ -57,30 +57,28 @@ describe('PolicyCodeView', () => {
     expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1)
   })
 
-  it('code includes policy name, effect, and version', () => {
-    renderCodeView({ name: 'my-policy', effect: 'deny', version: 3 })
+  it('code includes policy name, policy_type, and version', () => {
+    renderCodeView({ name: 'my-policy', policy_type: 'column_deny', version: 3 })
     fireEvent.click(screen.getByText('View as code'))
     fireEvent.click(screen.getByText('JSON'))
     const parsed = JSON.parse(document.querySelector('pre')!.textContent!)
     expect(parsed.name).toBe('my-policy')
-    expect(parsed.effect).toBe('deny')
+    expect(parsed.policy_type).toBe('column_deny')
     expect(parsed.version).toBe(3)
   })
 
-  it('obligations show flattened definition fields', () => {
-    const obl = makeObligation({
-      id: 'obl-1',
-      obligation_type: 'row_filter',
-      definition: { filter: 'tenant_id = 1' },
+  it('code includes targets and definition', () => {
+    const policy = makePolicy({
+      policy_type: 'row_filter',
+      targets: [{ schemas: ['public'], tables: ['orders'] }],
+      definition: { filter_expression: 'tenant = 1' },
     })
-    const policy = makePolicy({ obligations: [obl] })
     renderWithProviders(<PolicyCodeView policy={policy} assignments={[]} />)
     fireEvent.click(screen.getByText('View as code'))
     fireEvent.click(screen.getByText('JSON'))
     const parsed = JSON.parse(document.querySelector('pre')!.textContent!)
-    expect(parsed.obligations).toHaveLength(1)
-    expect(parsed.obligations[0].type).toBe('row_filter')
-    expect(parsed.obligations[0].filter).toBe('tenant_id = 1')
+    expect(parsed.targets).toHaveLength(1)
+    expect(parsed.definition.filter_expression).toBe('tenant = 1')
   })
 
   it('assignments show datasource and user names', () => {

admin-ui/src/components/PolicyCodeView.test.tsx

@@ -10,15 +10,12 @@ function buildCodeObject(policy: PolicyResponse, assignments: PolicyAssignmentRe
   return {
     id: policy.id,
     name: policy.name,
-    effect: policy.effect,
+    policy_type: policy.policy_type,
     description: policy.description ?? null,
     is_enabled: policy.is_enabled,
     version: policy.version,
-    obligations: (policy.obligations ?? []).map((o) => ({
-      id: o.id,
-      type: o.obligation_type,
-      ...o.definition,
-    })),
+    targets: policy.targets,
+    definition: policy.definition ?? null,
     assignments: assignments.map((a) => ({
       id: a.id,
       datasource_id: a.data_source_id,

admin-ui/src/components/PolicyCodeView.tsx

@@ -15,120 +15,110 @@ function renderForm(props: Partial<Parameters<typeof PolicyForm>[0]> = {}) {
   )
 }
 
-describe('PolicyForm — deny + column_mask validation', () => {
-  it('column_mask option is visible when effect is permit', async () => {
+describe('PolicyForm — policy type selector', () => {
+  it('renders all 5 policy type options', () => {
     renderForm()
-    // Add an obligation — effect defaults to permit
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const comboboxes = screen.getAllByRole('combobox')
-    // The obligation type select is the last combobox
-    const obligationTypeSelect = comboboxes[comboboxes.length - 1]
-    const options = Array.from(obligationTypeSelect.querySelectorAll('option')).map(o => o.value)
+    const select = screen.getAllByRole('combobox')[0]
+    const options = Array.from(select.querySelectorAll('option')).map((o) => o.value)
+    expect(options).toContain('row_filter')
     expect(options).toContain('column_mask')
+    expect(options).toContain('column_allow')
+    expect(options).toContain('column_deny')
+    expect(options).toContain('table_deny')
   })
 
-  it('column_mask option is hidden when effect is deny', async () => {
+  it('shows deny note when column_deny is selected', async () => {
     renderForm()
-    // Switch effect to deny
-    const effectSelect = screen.getAllByRole('combobox')[0]
-    await userEvent.selectOptions(effectSelect, 'deny')
-    // Add an obligation
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    // The obligation type select is the last combobox
-    const obligationTypeSelect = typeSelects[typeSelects.length - 1]
-    const options = Array.from(obligationTypeSelect.querySelectorAll('option')).map(o => o.value)
-    expect(options).not.toContain('column_mask')
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'column_deny')
+    expect(screen.getByText(/Deny policies short-circuit or strip columns/i)).toBeTruthy()
   })
 
-  it('switching effect to deny removes existing column_mask obligations', async () => {
+  it('shows filter expression field when row_filter is selected', async () => {
     renderForm()
-    // Add a column_mask obligation while effect is permit
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    const obligationTypeSelect = typeSelects[typeSelects.length - 1]
-    await userEvent.selectOptions(obligationTypeSelect, 'column_mask')
-    // Verify the column_mask obligation is shown
-    expect(screen.getByText('Obligation 1')).toBeTruthy()
-    // Switch effect to deny
-    const effectSelect = screen.getAllByRole('combobox')[0]
-    await userEvent.selectOptions(effectSelect, 'deny')
-    // The column_mask obligation should have been removed
-    expect(screen.queryByText('Obligation 1')).toBeNull()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'row_filter')
+    expect(screen.getByText(/Filter Expression/i)).toBeTruthy()
+    expect(screen.queryByText(/Mask Expression/i)).toBeNull()
   })
 
-  it('shows a note about column masking when effect is deny', async () => {
+  it('shows mask expression field when column_mask is selected', async () => {
     renderForm()
-    const effectSelect = screen.getAllByRole('combobox')[0]
-    await userEvent.selectOptions(effectSelect, 'deny')
-    expect(screen.getByText(/Column masking is not available on deny policies/i)).toBeTruthy()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'column_mask')
+    expect(screen.getByText(/Mask Expression/i)).toBeTruthy()
+    expect(screen.queryByText(/Filter Expression/i)).toBeNull()
+  })
+
+  it('hides definition fields when table_deny is selected', async () => {
+    renderForm()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'table_deny')
+    expect(screen.queryByText(/Filter Expression/i)).toBeNull()
+    expect(screen.queryByText(/Mask Expression/i)).toBeNull()
   })
 })
 
-describe('PolicyForm — object_access obligation type', () => {
-  it('object_access option is available in the type selector', async () => {
+describe('PolicyForm — targets', () => {
+  it('starts with one target entry', () => {
     renderForm()
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    const obligationTypeSelect = typeSelects[typeSelects.length - 1]
-    const options = Array.from(obligationTypeSelect.querySelectorAll('option')).map(o => o.value)
-    expect(options).toContain('object_access')
+    expect(screen.getByText('Target 1')).toBeTruthy()
   })
 
-  it('object_access is available on deny policies', async () => {
+  it('adds a target when "+ Add target" is clicked', async () => {
     renderForm()
-    const effectSelect = screen.getAllByRole('combobox')[0]
-    await userEvent.selectOptions(effectSelect, 'deny')
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    const obligationTypeSelect = typeSelects[typeSelects.length - 1]
-    const options = Array.from(obligationTypeSelect.querySelectorAll('option')).map(o => o.value)
-    expect(options).toContain('object_access')
+    await userEvent.click(screen.getByText('+ Add target'))
+    expect(screen.getByText('Target 2')).toBeTruthy()
   })
 
-  it('object_access shows schema field and optional table field', async () => {
+  it('removes a target when Remove is clicked', async () => {
     renderForm()
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    const obligationTypeSelect = typeSelects[typeSelects.length - 1]
-    await userEvent.selectOptions(obligationTypeSelect, 'object_access')
-    // Should show the hint text for object_access
-    expect(screen.getByText(/Hides the entire schema/i)).toBeTruthy()
-    // Table field should have "leave blank" placeholder hint
-    expect(document.body.textContent).toMatch(/optional/i)
+    await userEvent.click(screen.getByText('+ Add target'))
+    expect(screen.getByText('Target 2')).toBeTruthy()
+    const removeButtons = screen.getAllByText('Remove')
+    await userEvent.click(removeButtons[0])
+    expect(screen.queryByText('Target 2')).toBeNull()
+  })
+
+  it('shows columns field for column_allow', async () => {
+    renderForm()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'column_allow')
+    expect(screen.getByText('Columns')).toBeTruthy()
+  })
+
+  it('hides columns field for row_filter', async () => {
+    renderForm()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'row_filter')
+    expect(screen.queryByText('Columns')).toBeNull()
+  })
+
+  it('hides columns field for table_deny', async () => {
+    renderForm()
+    const select = screen.getAllByRole('combobox')[0]
+    await userEvent.selectOptions(select, 'table_deny')
+    expect(screen.queryByText('Columns')).toBeNull()
   })
 })
 
-describe('PolicyForm — submits object_access obligation correctly', () => {
-  it('omits table field when blank for schema-level deny', async () => {
+describe('PolicyForm — submission', () => {
+  it('calls onSubmit with policy_type and targets', async () => {
     const onSubmit = vi.fn().mockResolvedValue(undefined)
     renderForm({ onSubmit })
 
-    // Add object_access obligation
-    await userEvent.click(screen.getByText('+ Add obligation'))
-    const typeSelects = screen.getAllByRole('combobox')
-    await userEvent.selectOptions(typeSelects[typeSelects.length - 1], 'object_access')
-
-    // Set schema
-    const textboxes = screen.getAllByRole('textbox')
-    // Find the schema input (has placeholder 'analytics')
-    const schemaInput = textboxes.find(i => (i as HTMLInputElement).placeholder === 'analytics')
-    if (schemaInput) {
-      await userEvent.clear(schemaInput)
-      await userEvent.type(schemaInput, 'analytics')
-    }
+    // Name is required — fill it in
+    const nameInput = screen.getAllByRole('textbox')[0]
+    await userEvent.clear(nameInput)
+    await userEvent.type(nameInput, 'my-policy')
 
-    // Submit
     fireEvent.submit(document.querySelector('form')!)
-    // Wait for onSubmit to be called
-    await new Promise(r => setTimeout(r, 0))
+    await new Promise((r) => setTimeout(r, 0))
 
     if (onSubmit.mock.calls.length > 0) {
       const values = onSubmit.mock.calls[0][0]
-      const obl = values.obligations[0]
-      expect(obl.obligation_type).toBe('object_access')
-      expect(obl.definition.action).toBe('deny')
-      expect(obl.definition.table).toBeUndefined()
+      expect(values.policy_type).toBe('row_filter')
+      expect(Array.isArray(values.targets)).toBe(true)
     }
   })
 })