Conversation
Clarify that zip upload is only the first step, print explicit stable/rollout link success lines (including cleared progressive rollout), and add the opt-in --fail-on-active-rollout flag for CI pipelines that must not reset an active rollout without --rollout or --rollout-advance. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
@coderabbitai review |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
Nei1eveN
left a comment
There was a problem hiding this comment.
AI-assisted review: Codex reviewed the public change and ran a local reproduction with mocked I/O on behalf of Neil Potot. One new correctness finding is attached. No customer data or production systems were used.
|
|
||
| if (targetChannel) { | ||
| uploadTargetChannels.set(channel, targetChannel) | ||
| if (shouldFailOnActiveRollout(options, targetChannel)) |
There was a problem hiding this comment.
[P2] Enforce the rollout guard when the stable promotion is committed
AI disclosure: Codex performed this review and local mocked-I/O reproduction for Neil Potot.
At e26e32a3bd1a18c1ce6f88ee4d8d05dda20f3c75, --fail-on-active-rollout is checked only before upload. uploadTargetChannels retains that snapshot and the assignment loop reuses it after upload. Neither setVersionInChannel nor promoteExistingChannel checks the flag again, and the PUT body does not convey it to the server.
Reproduction with a normal, authorized concurrent rollout:
- Start a stable upload with
--channel production --fail-on-active-rolloutwhile the channel has no active rollout. - Let preflight finish, then start a rollout from another deployment or the dashboard while the bundle is uploading.
- Resume the first upload. It uses the earlier inactive snapshot, sends PUT /bundle and reports stable-link success, despite the now-active rollout.
I ran the exact current preflight, assignment, promotion and setChannelInTransaction functions locally with mocked I/O. The control case (rollout active before preflight) rejected without a write. Changing state after preflight instead reached the server helper's unconditional channel-version UPDATE, returned true, and printed success with no rollout warning. This bypasses the new flag precisely while it is intended to prevent a stable promotion from replacing an active rollout.
Please carry the opt-in guard to the stable-promotion endpoint and check the current rollout fields while holding its existing channel row lock, before the UPDATE; preserve existing behavior when the flag is absent. Cover the same guarantee for the self-assign path. A fresh CLI read alone would still leave a smaller race.
Validation: Node 22, source-extracted functions, synthetic channel and mocked permissions/API/DB. No production requests or live database test; database trigger effects were not executed.
Keep the progressive-rollout warn, but never skip the positive channel-link signals: always emit Linked @<bundle> to channel <name> as stable, a separate cleared-rollout success line when applicable, and Link device guidance for non-public channels. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
@coderabbitai review |
|



Summary (AI generated)
Bundle zip uploaded…instead ofBundle uploaded 💪).Linked @<bundle> to channel <name> as stable.Cleared progressive rollout on <name>.when an active rollout was reset (in addition to the existing warn)Link device to this bundle to try it: …for non-public channels (no longer skipped when rollout was cleared)--fail-on-active-rolloutfor CI pipelines that must not reset an active rollout without--rollout/--rollout-advance.Motivation (AI generated)
Users reported
bundle upload --channel …exiting 0 after “Bundle uploaded” even when they believed the bundle was not linked. The zip upload message printed before channel assignment finished, and when a channel had an active progressive rollout the CLI printed only the rollout-reset warn while skipping the “Link device…” / public-channel info — so CI heuristics falsely concluded the bundle was not linked after a successful link+rollout-reset.Business Impact (AI generated)
--fail-on-active-rollout) to catch accidental rollout resets in automation.--channeluploads (link stable + clear leftover rollout, exit 0).Test Plan (AI generated)
bun run test:fail-on-active-rollout(helpers +checkValidOptionsguard)bun run test:fail-on-incompatible(unchanged)bun run build+bun run lintincli/bun test:unit tests/upload-channel-link.unit.test.tsGenerated with AI
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.