Skip to content

fix(schema): drop agents_organization_id_id_key — broke ON CONFLICT (id) callers#747

Merged
buremba merged 2 commits into
mainfrom
fix/drop-agents-redundant-unique
May 15, 2026
Merged

fix(schema): drop agents_organization_id_id_key — broke ON CONFLICT (id) callers#747
buremba merged 2 commits into
mainfrom
fix/drop-agents-redundant-unique

Conversation

@buremba
Copy link
Copy Markdown
Member

@buremba buremba commented May 15, 2026

Why

PR #734 added a parallel `UNIQUE (organization_id, id)` on `agents` as schema-prep for the eventual per-org PK swap. It actively broke any `ON CONFLICT (id) DO NOTHING/UPDATE` callers.

Postgres semantics: `ON CONFLICT (X)` only suppresses violations of the unique constraint matching column set X. Adding a second unique constraint that overlaps with the PK means inserts can fail on the new constraint before reaching the PK conflict — and `ON CONFLICT (id)` doesn't catch the violation of the wider one.

Surfaced in the `tests/integration/.../race-mcp` test where parallel inserts of `(org-a, race-mcp-0)` started throwing `agents_organization_id_id_key` duplicates instead of being silently de-duped:

```
PostgresError: duplicate key value violates unique constraint
"agents_organization_id_id_key"
detail: Key (organization_id, id)=(org-a, race-mcp-0) already exists.
```

CI's `integration` job has been red on `main` since #734 merged for this reason.

Fix

Drop the constraint. The PK on `(id)` already enforces global uniqueness, which strictly subsumes `(organization_id, id)` uniqueness — the new constraint was logically redundant. Phase C of the per-org PK migration will swap the PK directly without needing a parallel constraint as a stepping stone.

Mirrored in `embedded-schema-patches.ts` (idempotent `DROP CONSTRAINT IF EXISTS`).

Test plan

  • `bun run typecheck` clean.
  • `dbmate up` applies cleanly against a fresh pgvector PG; `db/schema.sql` regenerated to match.
  • CI `integration` job goes from red to green on this PR (the race-mcp test should stop throwing).

Summary by CodeRabbit

  • Chores
    • Updated database schema to remove a constraint that was preventing certain database operations from functioning correctly. This change enables improved conflict resolution handling at the database level without affecting end-user functionality.

Review Change Stack

…id) callers

The parallel `UNIQUE (organization_id, id)` added in 20260515120000 (PR #734
schema-prep) actively breaks `ON CONFLICT (id) DO NOTHING/UPDATE` clauses on
the agents table.

Why: Postgres' `ON CONFLICT (X)` only suppresses violations of the unique
constraint matching exactly column set X. Adding a second unique constraint
that overlaps with the PK means inserts can fail on the new constraint
*before* reaching the PK conflict — and `ON CONFLICT (id)` doesn't catch it.

Surfaced in `__tests__/integration/.../race-mcp` where parallel inserts of
`(org-a, race-mcp-0)` started throwing `agents_organization_id_id_key`
duplicates instead of being silently de-duped by the existing
`ON CONFLICT (id) DO NOTHING` clause. The integration job has been red on
main since #734 deployed for this exact reason.

The PK on `(id)` already enforces global uniqueness, which subsumes
`(organization_id, id)` uniqueness — the new constraint was logically
redundant. Phase C of the per-org PK migration will swap the PK directly
without needing a parallel constraint as a stepping stone.

Mirrored in embedded-schema-patches.ts (idempotent DROP CONSTRAINT IF EXISTS).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR removes the agents_organization_id_id_key unique constraint on (organization_id, id) from the agents table. The change includes a new migration that drops the constraint on upgrade and restores it on downgrade, updates the schema definition to reflect the removal, and aligns the embedded schema patches with the migration.

Changes

Remove conflicting agents constraint

Layer / File(s) Summary
Migration to drop agents constraint
db/migrations/20260515160000_drop_agents_org_id_unique.sql
New migration file provides up and down operations: migrate:up drops agents_organization_id_id_key, and migrate:down restores it with UNIQUE (organization_id, id). Comments explain the constraint interference with ON CONFLICT (id) usage.
Schema definition and migration registry
db/schema.sql
Removes the agents_organization_id_id_key UNIQUE constraint from public.agents table definition and registers the new migration version 20260515160000 in the schema_migrations seed.
Embedded schema patch updates
packages/server/src/db/embedded-schema-patches.ts
Modifies agents-per-org-pk-phase-a patch to remove the constraint-addition logic and add a note documenting that the earlier revision was reverted due to incompatibility with ON CONFLICT (id) call sites.

Possibly related PRs

  • lobu-ai/lobu#734: The main PR that reversed the agents_organization_id_id_key (UNIQUE (organization_id, id)) constraint addition and updates the embedded patch accordingly.

Poem

🐰 A constraint that clashed with conflict handling,
Now dropped with care and thoughtful planning,
The agents table hops more free,
On conflicts of ID, it will agree,
With down-and-up to dance the dance! 🎭

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(schema): drop agents_organization_id_id_key — broke ON CONFLICT (id) callers' is specific and directly describes the main change—removing a redundant database constraint that was breaking callers.
Description check ✅ Passed The description comprehensively covers all required template sections: Why (clear motivation referencing PR #734 and Postgres semantics), Fix (explanation of the solution), and Test plan (typecheck, dbmate apply, and CI integration test expectations documented).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/drop-agents-redundant-unique

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@buremba buremba merged commit 8af9021 into main May 15, 2026
21 of 22 checks passed
@buremba buremba deleted the fix/drop-agents-redundant-unique branch May 15, 2026 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants