Repository navigation
feat(projects): enforce Project membership and retire the connector - #8590
mzxchandra wants to merge 82 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ty-enforcement # Conflicts: # apps/sim/lib/projects/__integration__/foundation.integration.ts
mzxchandra
changed the title
|
@cubic-dev-ai review this PR |
@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 161 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
There was a problem hiding this comment.
2 issues found across 161 files
Confidence score: 4/5
- The barrier in
backfill-repair.integration.tsblocks all eight workers ahead oftail, so the test cannot verify thattailreceives a notification. Block only one slow request. - In
lifecycle.ts, a local pub/sub subscriber error can make strict archive repair treat provider cleanup as unfinished even after it succeeds. Keep notification failures separate from provider-cleanup status.
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/projects/__integration__/backfill-repair.integration.ts">
<violation number="1" location="apps/sim/lib/projects/__integration__/backfill-repair.integration.ts:38">
P2: This barrier prevents `tail` from being notified: the first eight sorted workflows (`flow` and `slow-1` through `slow-7`) all block, leaving `tail` queued. Block only one slow request so another worker can reach `tail` before the test releases the barrier.</violation>
</file>
<file name="apps/sim/lib/workspaces/lifecycle.ts">
<violation number="1" location="apps/sim/lib/workspaces/lifecycle.ts:154">
P2: Strict archive repair currently treats MCP notification failures as unfinished provider cleanup. If a local pub/sub subscriber throws after provider cleanup succeeds, this branch rethrows the notification error, leaves the repair journal incomplete, and blocks migration completion; isolate best-effort MCP notification errors from the strict provider-cleanup result.</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
|
@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 162 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
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Lock and verify effective legacy memberships before materializing reviewed assignments and applying the shared detach policy. Reject resume reports whose code hash differs before changing their checkpoint. Isolate local pubsub subscriber failures so healthy archive subscribers still receive notifications. Extend existing CLI and real-database integration coverage for mixed memberships, concurrent and stale connector changes, unchanged mismatched reports, replay, and local subscriber delivery.
|
@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 165 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. |
There was a problem hiding this comment.
1 issue found across 166 files
Confidence score: 3/5
- In
reconcile-project-membership.ts, enforcement SQL can resolve to a shadow schema even though preflight checkspublic.*, so it may apply changes to the wrong schema. Qualify the enforcement references or constrainsearch_path.
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="packages/db/scripts/reconcile-project-membership.ts">
<violation number="1" location="packages/db/scripts/reconcile-project-membership.ts:27">
P1: The preflight checks `public.*`, but the enforcement SQL resolves unqualified table and function names through the database URL's `search_path`; a shadow schema can therefore receive the triggers or have its `project_workspace` dropped. Set this connection's search path to `public` before calling the helper, or qualify the helper's objects.</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
| logger.info('Nullable Project membership column prepared') | ||
| if (!process.argv.includes('--prepare') && state.workspace && state.project) { | ||
| // Drizzle cannot express lifecycle triggers; fresh push uses the same enforcement as migrations. | ||
| await enforceProjectMembership(sql) |
There was a problem hiding this comment.
P1: The preflight checks public.*, but the enforcement SQL resolves unqualified table and function names through the database URL's search_path; a shadow schema can therefore receive the triggers or have its project_workspace dropped. Set this connection's search path to public before calling the helper, or qualify the helper's objects.
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/scripts/reconcile-project-membership.ts, line 27:
<comment>The preflight checks `public.*`, but the enforcement SQL resolves unqualified table and function names through the database URL's `search_path`; a shadow schema can therefore receive the triggers or have its `project_workspace` dropped. Set this connection's search path to `public` before calling the helper, or qualify the helper's objects.</comment>
<file context>
@@ -17,34 +17,15 @@ try {
- logger.info('Nullable Project membership column prepared')
+ if (!process.argv.includes('--prepare') && state.workspace && state.project) {
+ // Drizzle cannot express lifecycle triggers; fresh push uses the same enforcement as migrations.
+ await enforceProjectMembership(sql)
+ logger.info('Project membership validation and lifecycle enforcement completed')
}
</file context>
| await enforceProjectMembership(sql) | |
| await sql.unsafe('SET search_path = public') | |
| await enforceProjectMembership(sql) |
This branch was successfully deployed
1 participant
Summary
workspace.project_id. Deploy feat(projects): move Project membership to the workspace column #8830 and verify that incompatible application tasks and background workers have drained before this release; the deployment preflight remains mandatory.0031_project_membershipin the existing TypeScript migration runner. Discover all environments, including populated columns; preserve legitimate Project identity, prefer columns over stale connector assignments, and reconcile unambiguous Project scope and archive state. Never unarchive environments or workflows.project_workspaceunder brief non-waiting table locks. Ordinary workflow writes get no Project trigger. Fresh schema push uses the same enforcement and preserves recovery state.Exceptional recovery runs from the exact release checkout on an authorized operator host with current release/drain evidence and the direct primary connection. After reviewing and completing any required preparation, the normal migration entrypoint can consume
PROJECT_BACKFILL_REVIEW_PATH, validate/enforce, and write its normal completion receipt. The ordinary deployment retry then needs no private manifest. Direct invocation does not execute the GitHub Actions AWS preflight, so release coordination and fresh drain verification are still required. Never manually insert a completion receipt.Type of Change
Testing
d0a9792707integrates expansionb7ff6b1a02, preserving changelog 0404 and expansion 0405; enforcement is now 0406 plus registered 0031. Fresh real migration, no schema drift, migration safety and 24 focused checks passed. Changelog fixture failure was reproduced before its atomic Project-aware seed/cleanup fix; the optional Redis suite collects and skips all 9 cases when Redis is unset. Hosted validation for this head is pending.ecfdf8b8e8: 81 PostgreSQL 17 checks passed across Project contract, expansion and schema-push suites; 46 real database/application checks passed across repair, foundation, organization detachment and connector lifecycle locking.0dfc7abc1fand the unchanged runner tobdf50b29c7.1e4499eebepassed all applicable jobs, including all eight database shards, the full mobile E2E matrix, desktop live tests, lint, unit tests and build. The separate desktop smoke workflow hit an Electron shutdown timeout, passed its internal retry, and is being rerun; overall validation is not yet clean. Local validation does not replace deployment drainage or production-scale observation.Checklist
test-auditauthoring gate; separate desktop rerun and latest review correction pending)