feat(api): Share membership tables with OSS and fix account-table drift#4671
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
size:XL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesMembership models, account schema parity, and service integration
Workflow revision version repair
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
note over Alembic,PostgreSQL: OSS upgrade: 0a1b2c3d4e5f
Alembic->>PostgreSQL: CREATE TABLE organization_members / workspace_members / project_members
Alembic->>PostgreSQL: DELETE api_keys WHERE project_id IS NULL
Alembic->>PostgreSQL: ALTER api_keys.project_id NOT NULL
Alembic->>PostgreSQL: Backfill projects.organization_id / workspace_id
Alembic->>PostgreSQL: DELETE orphan projects (no users guard)
Alembic->>PostgreSQL: COUNT(*) NULL check → RuntimeError if > 0
Alembic->>PostgreSQL: ALTER projects NOT NULL + ADD FK CASCADE
end
rect rgba(60, 179, 113, 0.5)
note over Alembic,PostgreSQL: OSS upgrade: 1b2c3d4e5f6a (backfill)
Alembic->>upgrade_membership_backfill: session
upgrade_membership_backfill->>PostgreSQL: Query orgs / workspaces / projects / memberships
upgrade_membership_backfill->>PostgreSQL: INSERT missing owner memberships
upgrade_membership_backfill->>PostgreSQL: INSERT missing invitation-derived memberships
end
rect rgba(210, 105, 30, 0.5)
note over Alembic,PostgreSQL: EE upgrade: 2c3d4e5f6a7b
Alembic->>PostgreSQL: ALTER api_keys.created_by_id nullable
Alembic->>PostgreSQL: DROP / RECREATE projects_workspace_id_fkey CASCADE
Alembic->>PostgreSQL: Backfill projects.organization_id / workspace_id via members
Alembic->>PostgreSQL: DELETE orphan projects (no users guard)
Alembic->>PostgreSQL: COUNT(*) NULL check → RuntimeError if > 0
Alembic->>PostgreSQL: ALTER projects NOT NULL
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b5fb7659-a52c-49ed-8bad-b932df0176f0
📒 Files selected for processing (7)
api/ee/databases/postgres/migrations/core/versions/2c3d4e5f6a7b_account_schema_parity.pyapi/ee/src/models/db_models.pyapi/oss/databases/postgres/migrations/core/data_migrations/memberships.pyapi/oss/databases/postgres/migrations/core/versions/0a1b2c3d4e5f_add_membership_tables_and_account_parity.pyapi/oss/databases/postgres/migrations/core/versions/1b2c3d4e5f6a_backfill_membership_rows.pyapi/oss/src/models/db_models.pyapi/oss/src/services/db_manager.py
Contributor
Railway Preview Environment
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/ee/databases/postgres/migrations/core/versions/2c3d4e5f6a7b_account_schema_parity.py (1)
48-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-deterministic organization selection when user belongs to multiple organizations.
The subquery uses
LIMIT 1withoutORDER BY, so the selectedorganization_iddepends on physical row ordering. For users with multiple organization memberships, different migration runs could assign the same project to different organizations.The workspace backfill at lines 60-69 correctly uses
ORDER BY w.created_atfor determinism—apply the same pattern here.Proposed fix to add deterministic ordering
op.execute( """ UPDATE projects p SET organization_id = ( SELECT om.organization_id FROM project_members pm JOIN organization_members om ON om.user_id = pm.user_id WHERE pm.project_id = p.id AND om.organization_id IS NOT NULL + ORDER BY om.created_at LIMIT 1 ) WHERE p.organization_id IS NULL """ )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 65c0f48c-2ff7-4ec5-b4b3-0d03cef027ba
📒 Files selected for processing (1)
api/ee/databases/postgres/migrations/core/versions/2c3d4e5f6a7b_account_schema_parity.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/oss/databases/postgres/migrations/core/versions/0a1b2c3d4e5f_add_membership_tables_and_account_parity.py (1)
39-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
ON DELETE CASCADEto the membershipuser_idFKs.
api/oss/src/models/db_models.pydeclaresOrganizationMemberDB.user_id,WorkspaceMemberDB.user_id, andProjectMemberDB.user_idwithondelete="CASCADE", but this migration creates all three constraints without it. That recreates schema drift immediately, and deleting a user will start failing once membership rows exist.Suggested fix
- sa.ForeignKeyConstraint(["user_id"], ["users.id"]), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"), @@ - sa.ForeignKeyConstraint(["user_id"], ["users.id"]), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"), @@ - sa.ForeignKeyConstraint(["user_id"], ["users.id"]), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"),Also applies to: 55-58, 72-73
🧹 Nitpick comments (2)
api/entrypoints/worker_events.py (1)
26-27: ⚡ Quick winExtract duplicate
MAXLEN_QUEUES_WEBHOOKSconstant to a shared location.Both files define
MAXLEN_QUEUES_WEBHOOKS = 100_000for the same stream ("queues:webhooks"). To prevent drift, extract this constant to a shared module (e.g.,api/oss/src/utils/constants.pyorapi/oss/src/core/webhooks/constants.py) and import it in both workers.
api/entrypoints/worker_events.py#L26-L27: replace local definition with import from shared constants module.api/entrypoints/worker_webhooks.py#L29-L30: replace local definition with import from shared constants module.api/oss/tests/legacy/conftest.py (1)
842-842: 💤 Low valueAdd trailing slashes to OSS API test secret paths for consistency.
The OSS API test files use
"secrets"without trailing slashes, while the SDK test files correctly use"secrets/"with trailing slashes to match the router definition atapi/oss/src/apis/fastapi/vault/router.py. FastAPI will redirect, but the inconsistency adds unnecessary redirects and could confuse maintainers.
api/oss/tests/legacy/conftest.py#L842-L842: Change"secrets"to"secrets/"inreset_llm_keysGET request.api/oss/tests/legacy/conftest.py#L859-L859: Change"secrets"to"secrets/"inset_valid_llm_keysPOST request.api/oss/tests/legacy/conftest.py#L880-L880: Change"secrets"to"secrets/"inset_invalid_llm_keysGET request.api/oss/tests/legacy/workflows/core/tests.py#L182-L182: Change"secrets"to"secrets/"intest_missing_provider_keyPOST request.api/oss/tests/legacy/workflows/core/tests.py#L226-L226: Change"secrets"to"secrets/"intest_available_provider_key_with_invalid_provider_keyPOST request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c612a217-4b41-441e-b67a-d4916c4c6da3
⛔ Files ignored due to path filters (4)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
api/ee/databases/postgres/migrations/core/versions/2c3d4e5f6a7b_account_schema_parity.pyapi/entrypoints/worker_evaluations.pyapi/entrypoints/worker_events.pyapi/entrypoints/worker_webhooks.pyapi/oss/databases/postgres/migrations/core/versions/0a1b2c3d4e5f_add_membership_tables_and_account_parity.pyapi/oss/src/core/events/streaming.pyapi/oss/src/core/tracing/streaming.pyapi/oss/src/services/db_manager.pyapi/oss/tests/legacy/conftest.pyapi/oss/tests/legacy/workflows/core/tests.pyapi/pyproject.tomlclients/python/pyproject.tomldocs/openapi-cleanup/research.mdhosting/kubernetes/helm/Chart.yamlsdks/python/oss/tests/legacy/new_tests/conftest.pysdks/python/oss/tests/legacy/new_tests/workflows/core/tests.pysdks/python/pyproject.tomlservices/pyproject.tomlweb/ee/package.jsonweb/oss/package.jsonweb/package.jsonweb/packages/agenta-api-client/package.json
✅ Files skipped from review due to trivial changes (10)
- api/pyproject.toml
- web/packages/agenta-api-client/package.json
- web/oss/package.json
- clients/python/pyproject.toml
- services/pyproject.toml
- sdks/python/pyproject.toml
- web/ee/package.json
- docs/openapi-cleanup/research.md
- hosting/kubernetes/helm/Chart.yaml
- web/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- api/oss/src/services/db_manager.py
- api/ee/databases/postgres/migrations/core/versions/2c3d4e5f6a7b_account_schema_parity.py
junaway
force-pushed
the
feat/oss-membership-tables
branch
from
9437bd2 to
339c787
Compare
dosubot
Bot
added
size:XXL
Moves OrganizationMemberDB/WorkspaceMemberDB/ProjectMemberDB from EE to the shared OSS models (EE re-exports), creates the three tables in the OSS chain with the exact EE shapes, backfills membership rows from owners and used invitations, and aligns api_keys/projects with the shared model in both chains. get_user_organizations now uses the membership join in both editions.
…atabases Fresh replays seed a default project (911e6034d05e) before any organization can exist, so the parity migrations' NULL-scope guard killed the chain on new deployments. Delete NULL-scope projects when the database has no users (the fresh-replay signature); real deployments keep failing loudly. The OSS org backfill now prefers the legacy singleton (slug oss-default, NULLS LAST so unslugged orgs sort after) before falling back to the oldest org, making adoption deterministic with 0, 1, or N orgs and NULL or set slugs.
Lifecycle actor columns carry no FKs by convention (settled in the phase-2 review). The parity direction reverses for this column: instead of EE gaining the OSS FK, OSS drops its legacy FK in the cleanup migration. Only the nullable-ification stays here. Editing the unshipped migration avoids shipping an FK that the next PR would immediately drop.
…the chain The v0.103.5 merge dropped b3c4d5e6f7a9 (repair_workflow_revision_versions), the head of the v0.103.5 core chain, and re-rooted the membership/parity migrations onto its parent a2b3c4d5e6f8. Any database that ran v0.103.5 is stamped at b3c4d5e6f7a9, so upgrading it into the stack failed with 'Can't locate revision b3c4d5e6f7a9'. Restore the migration (OSS + EE) and its shared data_migration verbatim, and re-root 0a1b2c3d4e5f / 2c3d4e5f6a7b onto b3c4d5e6f7a9 so the chain is a2b3c4d5e6f8 -> b3c4d5e6f7a9 -> membership.
The 911e6034d05e boot seed creates an empty default project before any org exists. On a populated DB the app already made a real default project per workspace at signup, so adopting the seed left two is_default projects in one workspace and the members view (which reads the default project) showed only the seed's lone member. Drop the unscoped empty seed when a scoped default already exists; the org keeps exactly one default.
junaway
force-pushed
the
feat/oss-membership-tables
branch
from
774c502 to
548913d
Compare
dosubot
Bot
added
size:XL
The data migration imported live ORM models, so a later model change could break replaying revision 1b2c3d4e5f6a on a fresh or delayed upgrade. Use local sa.table()/sa.column() definitions pinned to the schema at this revision instead. Backfill logic is unchanged.
junaway
approved these changes
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
OSS is hard single-org today. The first blocker is that OSS has no membership tables at all:
organization_members,workspace_members, andproject_membersexist only in EE, so OSS derives "which orgs does user X belong to" implicitly fromorganizations.owner_idplus used invitations. That only works with exactly one org. The account tables have also drifted between the two editions (details indocs/designs/oss-ee-convergence/assessment-a-oss-multi-org.md, sequencing step 1).Changes
Membership models move from
api/ee/src/models/db_models.pytoapi/oss/src/models/db_models.py; the EE file becomes a re-export so existing EE imports keep working.project_members.is_demomoves with the class (kept in OSS for schema parity).New OSS core migrations:
0a1b2c3d4e5fcreates the three membership tables with the exact shapes the EE database has (same columns, order, FKs, includingupdated_by_id).api_keys.project_idbecomes NOT NULL (rows with NULLproject_idare pre-project-scoping keys that cannot authenticate any scope, so they are deleted), the duplicatefk_projects_organization_id(SET NULL) is dropped in favor of the CASCADE FK that matches EE,fk_projects_workspace_idis renamed toprojects_workspace_id_fkey, and the redundantuq_projects_idis dropped.1b2c3d4e5f6abackfills membership rows: org owners becomeownermembers of their org and its workspaces/projects; users holding a used invitation become members of the invitation's project, workspace, and org with the invitation's role.New EE core migration
2c3d4e5f6a7bfixes EE-side drift:api_keys.created_by_idbecomes nullable. No FK is added: lifecycle actor columns carry no FKs by convention (phase-2 review decision); OSS drops its legacy FK on this column in the follow-up cleanup.Both chains also align
projectswith the shared model:organization_idandworkspace_idbecome NOT NULL (after backfill) and the workspace FK becomes ON DELETE CASCADE. The backfill works with 0, 1, or N organizations, slugged or not: the OSS org adoption prefers the legacy singleton (slugoss-default, backfilled earlier in the chain) and falls back to the oldest org. On fresh replays the chain seeds a default project (911e6034d05e) before any organization can exist, so NULL-scope projects on a user-less database are deleted (they are recreated per organization at signup); on databases with users both migrations still raise instead of guessing.Behavior change:
get_user_organizations()indb_manager.pynow uses the membership join in both editions. Before, the OSS branch returned[].Tests / notes
ruff formatandruff checkpass; all touched files compile.api_keyscolumn order.docs/designs/oss-ee-convergence/ee_core.txt), not from the model classes; the dump carriesupdated_by_idcolumns the models do not declare.🤖 Generated with Claude Code