fix(backend): don't raise a mirror compatibility event on guarded rollback#2482
Conversation
…lback Rolling a channel back to the baseline of an unresolved compatibility event re-enters on_channel_update, which raised a fresh mirror event (prev/cur swapped) while only the original auto-resolved — so the recommended remediation (roll back) created an endless chain of unresolved events. decideCompatibilityEvents now receives the channel's unresolved events and suppresses an event whose new bundle is a known unresolved baseline for the same channel+platform (a revert). Suppression only applies while disable_auto_update_under_native is ON: the update endpoint then refuses to serve a bundle below a device's native version, so devices that already installed the newer native build cannot receive the rolled-back bundle. With the guard off the mirror event is a real warning and is still raised. Channel-scoped, unresolved-only (an accepted incompatibility later reverted still raises). 4 new unit tests (suppress / guard-off / wrong-channel / unknown-baseline).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughThis PR extends the compatibility-events decision pipeline to suppress "mirror" incompatibility events when a version revert already has an unresolved event tracking that revert scenario. The decision logic now checks unresolved events during platform-specific decisions and skips emitting duplicate incompatibility events when the revert baseline matches an existing unresolved event's previous version. Trigger-side callers now load and pass unresolved events into the decision logic to enable this suppression. ChangesMirror-event suppression for compatibility-event reverts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 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 docstrings
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
…arded rollback - Revert the on_channel_update / compatibility_events trigger changes from this branch; the mirror-event suppression now lives in the backend-only PR #2482 (gated on disable_auto_update_under_native) so it can merge and deploy first. - Rollback confirm dialog now appends a warning when an affected channel has the downgrade guard off: devices already on a newer native build could receive the older bundle — suggest enabling the guard first.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c9a090b83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address review: also require the unresolved event's current_version_id to match the change's previous bundle, so only the exact mirror (700 -> 600 while 600 -> 700 is open) is suppressed; a non-inverse transition into a known baseline (800 -> 600) still raises its own event since its immediate baseline is 800. New unit test for that case.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/functions/_backend/triggers/compatibility_events.ts (1)
59-67: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAlign the
unresolvedEventsdoc block with the stricter direct-inverse rule.This comment still reads as “suppress whenever the new bundle matches an unresolved baseline,” but the implementation now suppresses only the exact inverse transition. Leaving the exported input docs broader than the code makes the new 800 -> 600 / open 600 -> 700 behavior look incorrect.
♻️ Suggested doc update
/** * Unresolved events already on file for this channel, used to recognize a - * REVERT: when the new default bundle is the `previous_version` of an - * unresolved event, users are being returned to the baseline they are - * already on, so no new (mirror) event should be raised — otherwise the - * recommended remediation (roll the channel back) would itself raise a fresh - * unresolved event, forever. + * REVERT: when the new default bundle is the `previous_version` of an + * unresolved event AND the platform's immediate previous bundle is that + * event's `current_version`, users are being returned to the baseline they + * are already on, so no new (mirror) event should be raised — otherwise the + * recommended remediation (roll the channel back) would itself raise a fresh + * unresolved event, forever. */Based on PR objectives, suppression should apply only to the direct inverse rollback transition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/_backend/triggers/compatibility_events.ts` around lines 59 - 67, The doc for unresolvedEvents is too broad; update it to state suppression only applies for the exact direct-inverse rollback of a recorded unresolved event. Explain that suppression occurs only when the new default bundle equals an UnresolvedCompatibilityEvent.previous_version AND the change's previous default equals that event's version (i.e., the change is the exact inverse of the original transition), and reference unresolvedEvents and the UnresolvedCompatibilityEvent fields (previous_version and version) in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/compatibility-events-decide.unit.test.ts`:
- Around line 243-269: Rename the function-scoped constant PKG_V8 to camelCase
pkgV8 and update its usages in this test, and make the test concurrency-safe by
changing the test declaration from it(...) to it.concurrent(...); specifically
update the local variable PKG_V8 -> pkgV8 where it's passed into bundle(...) and
keep the rest of the test logic (calls to decideCompatibilityEvents, newChannel,
bundle, expect) unchanged.
---
Outside diff comments:
In `@supabase/functions/_backend/triggers/compatibility_events.ts`:
- Around line 59-67: The doc for unresolvedEvents is too broad; update it to
state suppression only applies for the exact direct-inverse rollback of a
recorded unresolved event. Explain that suppression occurs only when the new
default bundle equals an UnresolvedCompatibilityEvent.previous_version AND the
change's previous default equals that event's version (i.e., the change is the
exact inverse of the original transition), and reference unresolvedEvents and
the UnresolvedCompatibilityEvent fields (previous_version and version) in the
comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 864b3515-f9e5-4c73-bbb5-6da9638a022d
📒 Files selected for processing (2)
supabase/functions/_backend/triggers/compatibility_events.tstests/compatibility-events-decide.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
Address review: the repo convention is it.concurrent for concurrency-safe unit tests, and SCREAMING_SNAKE_CASE is reserved for module-level constants. Applied to the 5 tests this PR adds (pre-existing tests left untouched to keep the diff scoped).
|
…Compatibility page (#2480) * feat(frontend): explain & fix native-incompatibility on the Dependencies page When the dependency diff is incompatible, show an actionable guidance panel (modeled on the incompatible-bundle email): what it means (OTA can't change native code, devices on the old native build may crash), how to fix it (rebuild with Capgo Builder — opens the sell deck — or ship a native build; roll the channel back to the last compatible bundle meanwhile), and a collapsible 'why native changes need an app-store update' explainer. Tracks builder_cta_compatibility_clicked. * refactor(frontend): move native-incompatibility guidance to the Compatibility page The per-bundle dependency-compare view was the wrong home for the 'this needs a native build' explainer. Move it to the app-level Compatibility page (/app/:app/compatibility), shown whenever the app has unresolved incompatibility events. Make the copy generic (no per-bundle name), keep the Capgo Builder CTA (sell deck) + rollback guidance + the collapsible 'why native changes need an app-store update' explainer. Revert the Dependencies page to its original state. * refactor(frontend): redesign compatibility fix-guidance panel - Cleaner, more readable layout: card header + two distinct fix-option cards (rebuild / roll back) instead of dense stacked paragraphs; bump body copy to text-sm. - The Builder CTA now opens the in-app Builds tab instead of the 5-slide presentation deck. - Add a docs link to the bundle-compatibility / disable-updates strategy section. * style(frontend): lighter neutral compatibility panel + make it collapsible - Drop the amber header band / border / icon circle for the standard neutral card palette (slate borders, white/slate-900 bg); keep just a small amber alert icon as the cue. - Make the whole guidance panel collapsible via a header toggle, persisting the collapsed/expanded choice in localStorage (default expanded). * feat(frontend): one-click channel rollback + live-updates compatibility docs link - 'Roll back the channel' card now has a primary 'Roll back for me' button that, after a confirm dialog listing the exact changes (channel → last compatible bundle), points each affected channel back to the previous bundle from its most recent unresolved compatibility event. Skips deleted channels/bundles; checks channel.promote_bundle per channel. 'Manage channels' stays as the secondary button. - Point the docs link at the live-updates compatibility guide. * fix: don't raise a mirror compatibility event on rollback + single toast Backend: rolling a channel back to the baseline of an unresolved compatibility event used to raise a fresh mirror event (prev/cur swapped) while only the original auto-resolved — so the recommended remediation created an endless chain of unresolved events. decideCompatibilityEvents now takes the channel's unresolved events and suppresses an event whose current bundle is a known unresolved baseline for that channel+platform (a revert); the original event still auto-resolves on the same pass. Suppression is channel-scoped and only applies while the event is unresolved (an accepted incompatibility later reverted still raises). Frontend: rollback now shows a single toast and re-refreshes at 4s/10s so the queued auto-resolution becomes visible without a manual reload. 3 new unit tests for the revert suppression. * refactor(frontend): split backend trigger fix to #2482 + warn on unguarded rollback - Revert the on_channel_update / compatibility_events trigger changes from this branch; the mirror-event suppression now lives in the backend-only PR #2482 (gated on disable_auto_update_under_native) so it can merge and deploy first. - Rollback confirm dialog now appends a warning when an affected channel has the downgrade guard off: devices already on a newer native build could receive the older bundle — suggest enabling the guard first. * fix(frontend): address hostile-review findings on the rollback flow - Track the 4s/10s post-rollback refresh timers and clear them on unmount (and on a subsequent rollback) instead of firing into a dead component. - Per-channel rollback outcome: partial failures now name the failed channel(s) and acknowledge which channels did roll back, instead of one generic error implying nothing happened. - Snapshot rollback targets at dialog-open and pass them to the handler, so the writes performed on confirm are exactly the changes the dialog displayed. - Soften the rollback card copy: 'the bundle it served before the incompatible change' instead of overpromising 'last compatible bundle / users stay safe' for chained-incompatibility cases; same for the rollbackTargets comment. * fix(frontend): only offer rollback for channels the user can roll back - Pre-check channel.promote_bundle per rollback target (request-id guarded watch) and gate the 'Roll back for me' CTA + confirm dialog on the permitted subset, so users without permission are never offered an action that would fail. - A late permission failure (revoked between dialog open and confirm) now skips just that channel and names it in the outcome toast instead of aborting the whole batch with an anonymous no-permission error. * style(frontend): DaisyUI d-btn classes for the guidance panel buttons Address review: interactive elements use DaisyUI primitives per AGENTS.md; replace the custom Tailwind button styling on the rebuild, rollback and manage-channels CTAs with d-btn variants.



Problem
Rolling a channel back to the baseline of an unresolved compatibility event (the remediation the Compatibility page recommends) re-enters
on_channel_update, which raised a fresh mirror event (prev/cur swapped) while only the original event auto-resolved. Net effect: every rollback produced the next unresolved event — an endless chain, and the Compatibility page never cleared.Fix
decideCompatibilityEventsnow receives the channel's unresolved events and suppresses an event whose new bundle is a baseline an unresolved event already says this channel's users are on (i.e., a revert). The original event still auto-resolves on the same pass via the existingdecideAutoResolves.Scoping guards:
disable_auto_update_under_nativeis ON (the column default). With the guard on, the update endpoint refuses to serve a bundle below a device's native version (update.ts→'Cannot revert under native version'), so devices that already installed the newer native build cannot receive the rolled-back bundle — the mirror event would warn about a delivery that can't happen. With the guard off, that delivery is possible and the mirror event is still raised.The handler loads the channel's unresolved events with one extra indexed query before deciding; the auto-resolve select gains
channel_idto match the shared row shape.Tests
4 new unit tests in
tests/compatibility-events-decide.unit.test.ts(pure decision logic, no Supabase needed): suppression on guarded revert, no suppression when the guard is off, no suppression for another channel's baseline, no suppression for an unknown baseline. 18/18 pass.Notes
bunx vitest run tests/compatibility-events-decide.unit.test.ts,oxlint supabase, andtsgobackend typecheck all green.Summary by CodeRabbit
Bug Fixes
Improvements
Tests