fix(channel): auto-unlink second bundle when disabling progressive rollout - #2821
fix(channel): auto-unlink second bundle when disabling progressive rollout#2821riderx wants to merge 1 commit into
Conversation
Leaving the second bundle attached after disable looked broken in the console; clear rollout_version (and pause state) across API, console, and CLI. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2c9396d8-bf81-433f-8aab-45eaae3a5d66) |
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 2/5
- In
supabase/functions/_backend/public/channel/post.ts, disabling rollout can fail for users who are allowed to update channel settings but not allowed to promote bundles, because the disable path writesrollout_versionand hits thechannel.promote_bundletrigger check; this can block legitimate disable operations in production — route disable through a permission-safe path (or adjust trigger/operation split) so it doesn’t require promote rights. - In
supabase/functions/_backend/public/channel/post.ts, a disable request that also sendsrolloutVersioncan skip the unlink logic and leave the rollout bundle attached, creating a state that contradicts the requested disable behavior — make disable always force the unlink branch regardless of whetherrolloutVersionis present.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/functions/_backend/public/channel/post.ts">
<violation number="1" location="supabase/functions/_backend/public/channel/post.ts:475">
P2: A disable request that also includes `rolloutVersion` still leaves the rollout bundle linked because this guard skips the unlink path whenever that field is present. If disabling always means unlinking the second bundle, the guard should depend on the disable/rollback/promote operations rather than the presence of `rolloutVersion`.</violation>
<violation number="2" location="supabase/functions/_backend/public/channel/post.ts:476">
P1: Disabling rollout now fails for callers who can update channel settings but cannot promote bundles: this assignment changes `rollout_version`, which the database trigger protects with `channel.promote_bundle`, while the endpoint skips that permission check for an omitted `rolloutVersion`. The authorization path should be aligned with the trigger while preserving the intended settings-only disable behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // Disabling progressive rollout unlinks the second bundle so both stay connected only while rollout is active. | ||
| if (body.rolloutEnabled === false && body.rolloutVersion === undefined && !body.rollback && !body.promoteToStable) { | ||
| channel.rollout_version = null |
There was a problem hiding this comment.
P1: Disabling rollout now fails for callers who can update channel settings but cannot promote bundles: this assignment changes rollout_version, which the database trigger protects with channel.promote_bundle, while the endpoint skips that permission check for an omitted rolloutVersion. The authorization path should be aligned with the trigger while preserving the intended settings-only disable behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_backend/public/channel/post.ts, line 476:
<comment>Disabling rollout now fails for callers who can update channel settings but cannot promote bundles: this assignment changes `rollout_version`, which the database trigger protects with `channel.promote_bundle`, while the endpoint skips that permission check for an omitted `rolloutVersion`. The authorization path should be aligned with the trigger while preserving the intended settings-only disable behavior.</comment>
<file context>
@@ -470,6 +470,14 @@ export async function post(c: Context<MiddlewareKeyVariables>, body: ChannelSet,
+
+ // Disabling progressive rollout unlinks the second bundle so both stay connected only while rollout is active.
+ if (body.rolloutEnabled === false && body.rolloutVersion === undefined && !body.rollback && !body.promoteToStable) {
+ channel.rollout_version = null
+ channel.rollout_paused_at = null
+ channel.rollout_pause_reason = null
</file context>
| } | ||
|
|
||
| // Disabling progressive rollout unlinks the second bundle so both stay connected only while rollout is active. | ||
| if (body.rolloutEnabled === false && body.rolloutVersion === undefined && !body.rollback && !body.promoteToStable) { |
There was a problem hiding this comment.
P2: A disable request that also includes rolloutVersion still leaves the rollout bundle linked because this guard skips the unlink path whenever that field is present. If disabling always means unlinking the second bundle, the guard should depend on the disable/rollback/promote operations rather than the presence of rolloutVersion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/functions/_backend/public/channel/post.ts, line 475:
<comment>A disable request that also includes `rolloutVersion` still leaves the rollout bundle linked because this guard skips the unlink path whenever that field is present. If disabling always means unlinking the second bundle, the guard should depend on the disable/rollback/promote operations rather than the presence of `rolloutVersion`.</comment>
<file context>
@@ -470,6 +470,14 @@ export async function post(c: Context<MiddlewareKeyVariables>, body: ChannelSet,
}
+
+ // Disabling progressive rollout unlinks the second bundle so both stay connected only while rollout is active.
+ if (body.rolloutEnabled === false && body.rolloutVersion === undefined && !body.rollback && !body.promoteToStable) {
+ channel.rollout_version = null
+ channel.rollout_paused_at = null
</file context>



Summary (AI generated)
rollout_version(the second bundle) and clear pause statePOST /channel), Capgo console channel page, and CLI--rollout-disableMotivation (AI generated)
Disabling progressive rollout previously only flipped
rollout_enabledtofalse, so both the stable and rollout bundles stayed linked to the channel. That looked odd and confusing in the console.Business Impact (AI generated)
Clearer channel state for users managing progressive rollouts; less accidental leftover second-bundle linkage after turning rollout off.
Test Plan (AI generated)
bunx vitest run tests/channel-post.unit.test.tsrollout_enabled: false/--rollout-disableclearsrollout_versionGenerated with AI
Made with Cursor