Repository navigation
feat(projects): move Project membership to the workspace column - #8830
mzxchandra wants to merge 20 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mzxchandra
changed the title
|
@greptile review |
|
@cubic review |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
|
The removed-helper warning is a required downstream integration step. #8830 and #8590 use createProjectRecord before inserting workspace.project_id in the same transaction; the connector-based createProjectForWorkspace must not be restored. #8609 and #8610 must merge the final #8590 head and reconcile their creation/fixture callers before merging. Their shared foundation files are still awaiting that parent synchronization, so local feature-specific conversions alone do not establish a green combined stack. Preserve the downstream account-deletion storage cleanup effects and deferred Project deletion when resolving those conflicts. |
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 26 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 30 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 31 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 30 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
| DO $$ BEGIN | ||
| IF NOT EXISTS (SELECT 1 FROM pg_constraint | ||
| WHERE conrelid = 'workspace'::regclass AND conname = 'workspace_project_id_project_id_fk') THEN | ||
| ALTER TABLE "workspace" ADD CONSTRAINT "workspace_project_id_project_id_fk" | ||
| FOREIGN KEY ("project_id") REFERENCES "project"("id") ON DELETE RESTRICT NOT VALID; | ||
| END IF; | ||
| END $$; |
There was a problem hiding this comment.
Removing the membership bridge breaks archive decisions during the rolling deployment. Start with a legacy parent assigned through project_workspace, then create its fork on the new release. The fork gets only workspace.projectId. If an older server archives the parent, its archiveProjectWithLastEnvironment query misses the fork and archives the Project even though the child is still active. The new release then refuses further forks and disconnects from that child.
Keep older lifecycle writers aware of new memberships, or stop them from serving these operations before column-only writes begin. Draining them only before #8590 is too late.
Knowledge Base Used: Database schema and migrations
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 31 files
Confidence score: 3/5
create.tsstops writingproject_workspace, so older app or worker versions can miss Project membership for newly created workspaces during the rollout. Keep the legacy assignment while versions are mixed.schema.tscreates the index before the post-push replay without a concurrent build, which can block writes on large workspace tables. Make the index build concurrent or prepare it through the replay.organization-workspaces.integration.tsomitsforked_from_workspace_id, so the detachment integration test fails whentransferWorkspaceProjectsselects it. Add the column to the fixture.foundation.integration.tscan release the legacy writer after matching an unrelated query, beforesplitForkProjectreaches the Project lock. Capture the detaching transaction’s backend PID and use it to identify the intended query.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/workspaces/create.ts">
<violation number="1" location="apps/sim/lib/workspaces/create.ts:125">
P2: New workspace creation no longer writes `project_workspace`, so older app or worker versions still running during this rollout see no Project membership for these workspaces. Preserve the legacy assignment until those versions have drained, or prevent them from handling these workspaces.</violation>
</file>
<file name="apps/sim/lib/workspaces/organization-workspaces.integration.ts">
<violation number="1" location="apps/sim/lib/workspaces/organization-workspaces.integration.ts:45">
P2: This fixture omits `forked_from_workspace_id`, which `transferWorkspaceProjects` selects through `getProjectEnvironmentSource`; the detachment integration test therefore fails with a missing-column error. Add the column to this table definition.</violation>
</file>
<file name="apps/sim/lib/projects/__integration__/foundation.integration.ts">
<violation number="1" location="apps/sim/lib/projects/__integration__/foundation.integration.ts:1404">
P2: This barrier can match an unrelated query blocked by `blockerPid`, allowing the test to release the legacy writer before `splitForkProject` reaches the Project lock. Capture the detaching transaction's backend PID and require that PID in the poll.
(Based on your team's feedback about scoping PostgreSQL lock barriers to the tested backend.)</violation>
</file>
<file name="packages/db/schema.ts">
<violation number="1" location="packages/db/schema.ts:2064">
P2: `db:push` creates this index before the post-push 0404 replay, and this declaration omits `.concurrently()`. Large workspace tables therefore get a blocking index build; mark the index concurrent or prepare it with 0404 before Drizzle push.</violation>
</file>
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
| ? lockedCreationContext.billedAccountUserId | ||
| : billedAccountUserId | ||
|
|
||
| const projectId = await createProjectRecord(tx, { |
There was a problem hiding this comment.
P2: New workspace creation no longer writes project_workspace, so older app or worker versions still running during this rollout see no Project membership for these workspaces. Preserve the legacy assignment until those versions have drained, or prevent them from handling these workspaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/workspaces/create.ts, line 125:
<comment>New workspace creation no longer writes `project_workspace`, so older app or worker versions still running during this rollout see no Project membership for these workspaces. Preserve the legacy assignment until those versions have drained, or prevent them from handling these workspaces.</comment>
<file context>
@@ -121,8 +122,16 @@ export async function createWorkspaceWithProjectInTransaction(
? lockedCreationContext.billedAccountUserId
: billedAccountUserId
+ const projectId = await createProjectRecord(tx, {
+ projectName,
+ name,
</file context>
| CREATE TABLE user_stats (user_id text PRIMARY KEY, storage_used_bytes bigint NOT NULL); | ||
| CREATE TABLE workspace ( | ||
| id text PRIMARY KEY, name text, owner_id text, organization_id text, workspace_mode text, | ||
| project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT, |
There was a problem hiding this comment.
P2: This fixture omits forked_from_workspace_id, which transferWorkspaceProjects selects through getProjectEnvironmentSource; the detachment integration test therefore fails with a missing-column error. Add the column to this table definition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/workspaces/organization-workspaces.integration.ts, line 45:
<comment>This fixture omits `forked_from_workspace_id`, which `transferWorkspaceProjects` selects through `getProjectEnvironmentSource`; the detachment integration test therefore fails with a missing-column error. Add the column to this table definition.</comment>
<file context>
@@ -26,15 +26,23 @@ const database = drizzle(connection, { schema })
CREATE TABLE user_stats (user_id text PRIMARY KEY, storage_used_bytes bigint NOT NULL);
CREATE TABLE workspace (
id text PRIMARY KEY, name text, owner_id text, organization_id text, workspace_mode text,
+ project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT,
billed_account_user_id text, allow_personal_api_keys boolean DEFAULT true,
archived_at timestamp, organization_assigned_at timestamp, updated_at timestamp,
</file context>
| project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT, | |
| project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT, | |
| forked_from_workspace_id text, |
| (error: unknown) => error | ||
| ) | ||
| try { | ||
| await waitUntilBlockedBy(blocker, "lock='project'") |
There was a problem hiding this comment.
P2: This barrier can match an unrelated query blocked by blockerPid, allowing the test to release the legacy writer before splitForkProject reaches the Project lock. Capture the detaching transaction's backend PID and require that PID in the poll.
(Based on your team's feedback about scoping PostgreSQL lock barriers to the tested backend.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/projects/__integration__/foundation.integration.ts, line 1404:
<comment>This barrier can match an unrelated query blocked by `blockerPid`, allowing the test to release the legacy writer before `splitForkProject` reaches the Project lock. Capture the detaching transaction's backend PID and require that PID in the poll.
(Based on your team's feedback about scoping PostgreSQL lock barriers to the tested backend.) </comment>
<file context>
@@ -1173,12 +1360,62 @@ describe('Project foundation at the database and application boundary', () => {
+ (error: unknown) => error
+ )
+ try {
+ await waitUntilBlockedBy(blocker, "lock='project'")
+ } finally {
+ release.resolve()
</file context>
| (table) => ({ | ||
| ownerIdIdx: index('workspace_owner_id_idx').on(table.ownerId), | ||
| organizationIdIdx: index('workspace_organization_id_idx').on(table.organizationId), | ||
| projectIdIdx: index('workspace_project_id_id_idx').on(table.projectId, table.id), |
There was a problem hiding this comment.
P2: db:push creates this index before the post-push 0404 replay, and this declaration omits .concurrently(). Large workspace tables therefore get a blocking index build; mark the index concurrent or prepare it with 0404 before Drizzle push.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/db/schema.ts, line 2064:
<comment>`db:push` creates this index before the post-push 0404 replay, and this declaration omits `.concurrently()`. Large workspace tables therefore get a blocking index build; mark the index concurrent or prepare it with 0404 before Drizzle push.</comment>
<file context>
@@ -2059,6 +2061,7 @@ export const workspace = pgTable(
(table) => ({
ownerIdIdx: index('workspace_owner_id_idx').on(table.ownerId),
organizationIdIdx: index('workspace_organization_id_idx').on(table.organizationId),
+ projectIdIdx: index('workspace_project_id_id_idx').on(table.projectId, table.id),
nonNegativeStorage: check(
'workspace_storage_used_bytes_non_negative',
</file context>
| projectIdIdx: index('workspace_project_id_id_idx').on(table.projectId, table.id), | |
| projectIdIdx: index('workspace_project_id_id_idx').on(table.projectId, table.id).concurrently(), |
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 31 files
Confidence score: 3/5
environment-source.tscan return the wrong environment when an older app instance reassigns onlyproject_workspaceandworkspace.project_idis already set. Keep both representations synchronized until old writers are gone.organization-workspaces.integration.tsno longer createsproject_workspace, but the reader can query the deployed public table through the test schema’ssearch_path, causing detachment to fail. Update the fixture so the table is available to the test.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/workspaces/organization-workspaces.integration.ts">
<violation number="1" location="apps/sim/lib/workspaces/organization-workspaces.integration.ts:45">
P2: This fixture no longer creates `project_workspace`, but `getProjectEnvironmentSource` detects the deployed public table and queries it through the test schema's `search_path`; detachment then fails with `relation does not exist`. Keep the legacy table in this fixture until the fallback is retired.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
| CREATE TABLE user_stats (user_id text PRIMARY KEY, storage_used_bytes bigint NOT NULL); | ||
| CREATE TABLE workspace ( | ||
| id text PRIMARY KEY, name text, owner_id text, organization_id text, workspace_mode text, | ||
| project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT, |
There was a problem hiding this comment.
P2: This fixture no longer creates project_workspace, but getProjectEnvironmentSource detects the deployed public table and queries it through the test schema's search_path; detachment then fails with relation does not exist. Keep the legacy table in this fixture until the fallback is retired.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/workspaces/organization-workspaces.integration.ts, line 45:
<comment>This fixture no longer creates `project_workspace`, but `getProjectEnvironmentSource` detects the deployed public table and queries it through the test schema's `search_path`; detachment then fails with `relation does not exist`. Keep the legacy table in this fixture until the fallback is retired.</comment>
<file context>
@@ -26,15 +26,23 @@ const database = drizzle(connection, { schema })
CREATE TABLE user_stats (user_id text PRIMARY KEY, storage_used_bytes bigint NOT NULL);
CREATE TABLE workspace (
id text PRIMARY KEY, name text, owner_id text, organization_id text, workspace_mode text,
+ project_id text NOT NULL REFERENCES project(id) ON DELETE RESTRICT,
billed_account_user_id text, allow_personal_api_keys boolean DEFAULT true,
archived_at timestamp, organization_assigned_at timestamp, updated_at timestamp,
</file context>
This branch was successfully deployed
2 participants
Summary
workspace.project_id. New workspace creation and reassignment write the column; compatibility reads prefer the column and fall back to existing connector assignments when it is null.Type of Change
Testing
Checklist
test-auditauthoring gate)